Skip to content

Commit 9357beb

Browse files
committed
Change to ULP-aware to handle floating-point precision differences
Signed-off-by: Lantao Jin <ltjin@amazon.com>
1 parent 3743dc5 commit 9357beb

2 files changed

Lines changed: 24 additions & 27 deletions

File tree

integ-test/src/test/java/org/opensearch/sql/calcite/tpch/CalcitePPLTpchIT.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,13 @@
55

66
package org.opensearch.sql.calcite.tpch;
77

8-
import static org.opensearch.sql.util.MatcherUtils.*;
8+
import static org.opensearch.sql.util.MatcherUtils.assertJsonEquals;
9+
import static org.opensearch.sql.util.MatcherUtils.closeTo;
10+
import static org.opensearch.sql.util.MatcherUtils.rows;
11+
import static org.opensearch.sql.util.MatcherUtils.schema;
12+
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
13+
import static org.opensearch.sql.util.MatcherUtils.verifyNumOfRows;
14+
import static org.opensearch.sql.util.MatcherUtils.verifySchemaInOrder;
915

1016
import java.io.IOException;
1117
import java.util.Locale;
@@ -56,9 +62,7 @@ public void testQ1() throws IOException {
5662
"F",
5763
37474,
5864
isPushdownDisabled() ? 37569624.63999998 : 37569624.64,
59-
isPushdownDisabled()
60-
? 35676192.096999995
61-
: approx(35676192.097, 1e-6), // workaround for arm64
65+
isPushdownDisabled() ? 35676192.096999995 : 35676192.097,
6266
isPushdownDisabled() ? 37101416.22242404 : 37101416.222424,
6367
25.354533152909337,
6468
isPushdownDisabled() ? 25419.231826792948 : 25419.231826792962,

integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@
4242

4343
public class MatcherUtils {
4444

45+
/** Absolute tolerance floor for {@link #closeTo} numeric comparisons. */
46+
private static final double ABSOLUTE_TOLERANCE = 1e-10;
47+
48+
/** Number of ULPs tolerated by {@link #closeTo} to absorb platform-dependent rounding. */
49+
private static final int ULP_TOLERANCE_FACTOR = 4;
50+
4551
private static final Logger LOG = LogManager.getLogger();
4652
private static final ObjectMapper JSON_MAPPER = new ObjectMapper();
4753

@@ -301,22 +307,7 @@ protected boolean matchesSafely(JSONArray array) {
301307
};
302308
}
303309

304-
public static Approx approx(double value, double error) {
305-
return new Approx(value, error);
306-
}
307-
308-
public static class Approx {
309-
final double value;
310-
final double error;
311-
312-
Approx(double value, double error) {
313-
this.value = value;
314-
this.error = error;
315-
}
316-
}
317-
318310
public static TypeSafeMatcher<JSONArray> closeTo(Object... values) {
319-
final double defaultError = 1e-10;
320311
return new TypeSafeMatcher<JSONArray>() {
321312
@Override
322313
protected boolean matchesSafely(JSONArray item) {
@@ -329,13 +320,7 @@ protected boolean matchesSafely(JSONArray item) {
329320
for (int i = 0; i < actualValues.size(); i++) {
330321
Object actual = actualValues.get(i);
331322
Object expected = expectedValues.get(i);
332-
if (expected instanceof Approx) {
333-
if (!(actual instanceof Number)
334-
|| Math.abs(((Number) actual).doubleValue() - ((Approx) expected).value)
335-
> ((Approx) expected).error) {
336-
return false;
337-
}
338-
} else if (actual instanceof Number && expected instanceof Number) {
323+
if (actual instanceof Number && expected instanceof Number) {
339324
if (!valuesAreClose((Number) actual, (Number) expected)) {
340325
return false;
341326
}
@@ -351,8 +336,16 @@ public void describeTo(Description description) {
351336
description.appendText(Arrays.toString(values));
352337
}
353338

339+
/**
340+
* ULP-aware comparison: tolerates up to {@link #ULP_TOLERANCE_FACTOR} ULPs or {@link
341+
* #ABSOLUTE_TOLERANCE}, whichever is larger.
342+
*/
354343
private boolean valuesAreClose(Number v1, Number v2) {
355-
return Math.abs(v1.doubleValue() - v2.doubleValue()) <= defaultError;
344+
double d1 = v1.doubleValue();
345+
double d2 = v2.doubleValue();
346+
double diff = Math.abs(d1 - d2);
347+
double ulpTolerance = ULP_TOLERANCE_FACTOR * Math.max(Math.ulp(d1), Math.ulp(d2));
348+
return diff <= Math.max(ABSOLUTE_TOLERANCE, ulpTolerance);
356349
}
357350
};
358351
}

0 commit comments

Comments
 (0)