[parijat1222q]

Summary

Fixes MATH-1696.

EnumeratedRealDistribution allows unsorted values and duplicate masses, but its quantile method currently accumulates probabilities in input order. For values [7, 3, -1, 3] with equal weights, inverseCumulativeProbability(0) returns 7 instead of -1, and inverseCumulativeProbability(1) returns 3 instead of 7.

Sort the copied PMF only inside inverseCumulativeProbability. Stored input order and seeded sampling remain unchanged. No constructor, public API or sampling change.

Three regression tests cover both constructors, duplicate masses, zero-weight outliers, support endpoints, and unchanged seeded sampling after a quantile query. The two unsorted-input tests fail against unchanged production code. Also reproduced on the published 3.3 jar.

Validation

Linux, JDK 17:

• Fixed target class: 15 tests pass.
• Complete split local test validation: 3,826 cases, zero final failures/errors, 33 existing skips. The repository's configured retry policy was unchanged; 48 cases were reported as flakes and passed on configured retries.
• All module verification phases completed, with zero Checkstyle violations. Existing warning-only PMD and coverage diagnostics remain; no claim of 100% coverage.
• Javadoc generation passes with UTF-8 locale.
• git diff --check passes.

The initial uninterrupted mvn clean verify exceeded the execution time limit. Tests and verification were completed in separate module/phase runs. The default legacy test execution completed, then the separate configured flaky-tests execution was rerun to completion using mvn -f commons-math-legacy/pom.xml surefire:test@flaky-tests. This is split validation, not a claim that one uninterrupted default build completed. No test, assertion, retry setting or check configuration was changed.

AI disclosure: This PR description and the fix/tests were generated with AI assistance (Instinct) and checked with the validation above.

[aherbert]

Thanks for the big report. I suggested a different approach. Please verify.

Note the EnumeratedIntegerDistribution follows the same pattern. It does not have this bug as it does not override inverseCumulativeProbability. It delegates to a binary search within a bracket using repeat calls to cumulative probability which will be slow due to repeat passes over the entire data. A change to add a single pass inverseCumulativeProbability would require a sorted inner distribution and could be supported the same way.

[aherbert]

Sorting the PMF on each call to inverseCumulativeProbability is a large performance overhead. The sorted list should be computed once.

The innerDistribution is private and constructed by iterating over a Map of value -> count pairs. This is a LinkedHashMap. If this is changed to TreeMap then the inner distribution will be created with a sorted set of values. This fixes the case where the values constructor is called (equal weights).

    public EnumeratedRealDistribution(final double[] data) {
        final Map<Double, Integer> dataMap = new TreeMap<>();
        // ...

For the other case (values and counts), the List of samples can be sorted:

public EnumeratedRealDistribution(final double[] singletons,
                                  final double[] probabilities)
   throws /* lots of exceptions */ {
        final List<Pair<Double, Double>> samples = createDistribution(singletons, probabilities);
        samples.sort(Comparator.comparingDouble(Pair::getKey));
        innerDistribution = new EnumeratedDistribution<>(samples);
}

The modifications above fix your new tests.

[parijat1222q]

Thanks, I verified the constructor-sorting approach and agree it is better here. It avoids sorting on every quantile call without an extra cached PMF, and keeps CDF and inverse-CDF accumulation in the same order. I've added a regression for that consistency case. The legacy module tests and verification pass. Sorting changes seeded sample sequences and floating-point accumulation order; I have left EnumeratedIntegerDistribution unchanged.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.23%. Comparing base (f554608) to head (15a3a63).
⚠️ Report is 414 commits behind head on master.

Files with missing lines	Patch %	Lines
...egacy/distribution/EnumeratedRealDistribution.java	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #353      +/-   ##
============================================
+ Coverage     86.54%   87.23%   +0.68%     
+ Complexity     9787       91    -9696     
============================================
  Files           532      499      -33     
  Lines         35516    33534    -1982     
  Branches       6194     5853     -341     
============================================
- Hits          30738    29252    -1486     
+ Misses         3518     3160     -358     
+ Partials       1260     1122     -138     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.