Reduce max scale to preserve numeric accuracy

Author: JonasKunz

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

@@ -29,7 +29,8 @@ public class ExponentialHistogramUtils {
     /** The number of bits used to represent the exponent of IEEE 754 double precision number. */
     private static final int EXPONENT_WIDTH = 11;
 
-    private static final double LOG_BASE2_E = 1D / Math.log(2);
+    private static final double LN_2 = Math.log(2);
+    private static final double LOG_BASE2_E = 1D / LN_2;
 
     // Magic number, computed via log(4/3)/log(2^(2^-64)), but exact
     private static final long SCALE_UP_64_OFFSET = 7656090530189244512L;
@@ -82,35 +83,21 @@ public static int getMaximumScaleIncrease(long index) {
     }
 
     public static double getUpperBucketBoundary(long index, int scale) {
-        long nextIndex = index;
-        if (index < Long.MAX_VALUE) {
-            nextIndex++;
-        }
-        return getLowerBucketBoundary(nextIndex, scale);
+        return getLowerBucketBoundary(index + 1, scale);
     }
 
     public static double getLowerBucketBoundary(long index, int scale) {
-        // TODO: handle numeric limits, implement exact algorithms with 128 bit precision
-        double inverseFactor = Math.pow(2, -scale);
-        return Math.pow(2, inverseFactor * index);
+        // TODO: handle numeric limits, implement by splitting the index into two 32 bit integers
+        double inverseFactor = Math.scalb(LN_2, -scale);
+        return Math.exp(inverseFactor * index);
     }
 
     public static double getPointOfLeastRelativeError(long bucketIndex, int scale) {
         // TODO: handle numeric limits, implement exact algorithms with 128 bit precision
-        double inverseFactor = Math.pow(2, -scale);
-        return Math.pow(2, inverseFactor * (bucketIndex + 1 / 3.0));
+        double inverseFactor = Math.scalb(LN_2, -scale);
+        return Math.exp(inverseFactor * (bucketIndex + 1/3.0));
     }
 
-    /**
-     * Compute the index for the given value.
-     *
-     * <p>The algorithm to retrieve the index is specified in the <a
-     * href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#exponential-buckets">OpenTelemetry
-     * specification</a>.
-     *
-     * @param value Measured value (must be non-zero).
-     * @return the index of the bucket which the value maps to.
-     */
     static long computeIndex(double value, int scale) {
         double absValue = Math.abs(value);
         // For positive scales, compute the index by logarithm, which is simpler but may be
@@ -135,7 +122,8 @@ static long computeIndex(double value, int scale) {
      *     Scales: Use the Logarithm Function</a>
      */
     private static long getIndexByLogarithm(double value, int scale) {
-        return (long) Math.ceil(Math.log(value) * computeScaleFactor(scale)) - 1;
+        double scaleFactor = Math.scalb(LOG_BASE2_E, scale);
+        return (long) Math.ceil(Math.scalb(Math.log(value) * LOG_BASE2_E, scale)) - 1;
     }
 
     /**
@@ -159,7 +147,4 @@ private static long mapToIndexScaleZero(double value) {
         return ieeeExponent;
     }
 
-    private static double computeScaleFactor(int scale) {
-        return Math.scalb(LOG_BASE2_E, scale);
-    }
 }

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

@@ -11,9 +11,9 @@
 
 public final class FixedSizeExponentialHistogram implements ExponentialHistogramBuilder, ExponentialHistogram {
 
-    // scale of 52 is the largest scale being able to represent the smallest and largest double numbers
-    // while giving a relative error less
-    public static final int DEFAULT_BUCKET_SCALE = 52;
+    // scale of 38 is the largest scale where the index computation doesn't suffer much rounding
+    // if we want to use something larger, we'll have to rework the math
+    public static final int DEFAULT_BUCKET_SCALE = 38;
 
     private final long[] bucketIndices;
     private final long[] bucketCounts;

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

@@ -0,0 +1,33 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.exponentialhistogram;
+
+import org.elasticsearch.test.ESTestCase;
+
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.lessThanOrEqualTo;
+
+public class ExponentialHistogramGeneratorTests extends ESTestCase {
+
+    public void testVeryLargeValue() {
+        double value =  Double.MAX_VALUE/10;
+        ExponentialHistogram histo = ExponentialHistogramGenerator.createFor(value);
+
+        long index = histo.positiveBuckets().peekIndex();
+        int scale = histo.scale();
+
+        double lowerBound = ExponentialHistogramUtils.getLowerBucketBoundary(index, scale);
+        double upperBound = ExponentialHistogramUtils.getUpperBucketBoundary(index, scale);
+
+        assertThat("Lower bucket boundary should be smaller than value", lowerBound, lessThanOrEqualTo(value));
+        assertThat("Upper bucket boundary should be greater than value", upperBound, greaterThanOrEqualTo(value));
+    }
+
+}