diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramQuantile.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramQuantile.java index 218873982b1b3..bcac4bfb0907f 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramQuantile.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramQuantile.java @@ -34,6 +34,7 @@ public class ExponentialHistogramQuantile { * It returns the value of the element at rank {@code max(0, min(n - 1, (quantile * (n + 1)) - 1))}, where n is the total number of * values and rank starts at 0. If the rank is fractional, the result is linearly interpolated from the values of the two * neighboring ranks. + * The result is clamped to the histogram's minimum and maximum values. * * @param histo the histogram representing the distribution * @param quantile the quantile to query, in the range [0, 1] @@ -60,6 +61,8 @@ public static double getQuantile(ExponentialHistogram histo, double quantile) { double upperFactor = exactRank - lowerRank; ValueAndPreviousValue values = getElementAtRank(histo, upperRank); + // Ensure we don't return values outside the histogram's range + values = values.clampTo(histo.min(), histo.max()); double result; if (lowerRank == upperRank) { @@ -158,8 +161,13 @@ private static ValueAndPreviousValue getBucketMidpointForRank(BucketIterator buc * @param valueAtRank the value at the desired rank */ private record ValueAndPreviousValue(double valueAtPreviousRank, double valueAtRank) { + ValueAndPreviousValue negateAndSwap() { return new ValueAndPreviousValue(-valueAtRank, -valueAtPreviousRank); } + + ValueAndPreviousValue clampTo(double min, double max) { + return new ValueAndPreviousValue(Math.clamp(valueAtPreviousRank, min, max), Math.clamp(valueAtRank, min, max)); + } } } diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/QuantileAccuracyTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/QuantileAccuracyTests.java index 141a9fd5b1fb0..2991fd96e6b3d 100644 --- a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/QuantileAccuracyTests.java +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/QuantileAccuracyTests.java @@ -63,6 +63,44 @@ public void testNoNegativeZeroReturned() { assertThat(median, equalTo(0.0)); } + public void testPercentilesClampedToMinMax() { + ExponentialHistogram histogram = createAutoReleasedHistogram( + b -> b.scale(0).setNegativeBucket(1, 1).setPositiveBucket(1, 1).max(0.00001).min(-0.00002) + ); + double p0 = ExponentialHistogramQuantile.getQuantile(histogram, 0.0); + double p100 = ExponentialHistogramQuantile.getQuantile(histogram, 1.0); + assertThat(p0, equalTo(-0.00002)); + assertThat(p100, equalTo(0.00001)); + } + + public void testMinMaxClampedPercentileAccuracy() { + ExponentialHistogram histogram = createAutoReleasedHistogram( + b -> b.scale(0) + .setPositiveBucket(0, 1) // bucket 0 covers (1, 2] + .setPositiveBucket(1, 1) // bucket 1 covers (2, 4] + .min(1.1) + .max(2.1) + ); + + // The 0.5 percentile linearly interpolates between the two buckets. + // For the (1, 2] bucket, the point of least relative error will be used (1.33333) + // For the (2, 4] bucket, the max of the histogram should be used instead (2.1) + double expectedResult = (4.0 / 3 + 2.1) / 2; + double p50 = ExponentialHistogramQuantile.getQuantile(histogram, 0.5); + assertThat(p50, equalTo(expectedResult)); + + // Test the same for min instead of max + histogram = createAutoReleasedHistogram( + b -> b.scale(0) + .setNegativeBucket(0, 1) // bucket 0 covers (1, 2] + .setNegativeBucket(1, 1) // bucket 1 covers (2, 4] + .min(-2.1) + .max(-1.1) + ); + p50 = ExponentialHistogramQuantile.getQuantile(histogram, 0.5); + assertThat(p50, equalTo(-expectedResult)); + } + public void testUniformDistribution() { testDistributionQuantileAccuracy(new UniformRealDistribution(new Well19937c(randomInt()), 0, 100)); }