Clamp before interpolation

Author: JonasKunz

libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramQuantile.java

@@ -61,14 +61,16 @@ 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) {
             result = values.valueAtRank();
         } else {
             result = values.valueAtPreviousRank() * (1 - upperFactor) + values.valueAtRank() * upperFactor;
         }
-        return removeNegativeZero(Math.clamp(result, histo.min(), histo.max()));
+        return removeNegativeZero(result);
     }
 
     private static double removeNegativeZero(double result) {
@@ -159,8 +161,16 @@ 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)
+            );
+        }
     }
 }

libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/QuantileAccuracyTests.java

@@ -73,6 +73,23 @@ public void testPercentilesClampedToMinMax() {
         assertThat(p99, equalTo(0.00001));
     }
 
+    public void testMinMaxClampedPercentileaccurace() {
+        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 (1, 2] 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));
+    }
+
     public void testUniformDistribution() {
         testDistributionQuantileAccuracy(new UniformRealDistribution(new Well19937c(randomInt()), 0, 100));
     }