From 3e6ab1114f0349d40bdf96142d96e9990ce9ea1e Mon Sep 17 00:00:00 2001 From: John Grimes Date: Tue, 16 Jul 2024 11:34:42 +1000 Subject: [PATCH] Update boundary functions implementation to use significant figures --- .../sql/boundary/DecimalBoundaryFunction.java | 82 ++++--- .../boundary/DecimalBoundaryFunctionTest.java | 206 +++++++++++++----- 2 files changed, 210 insertions(+), 78 deletions(-) diff --git a/fhirpath/src/main/java/au/csiro/pathling/sql/boundary/DecimalBoundaryFunction.java b/fhirpath/src/main/java/au/csiro/pathling/sql/boundary/DecimalBoundaryFunction.java index ac0c2f2ea3..0f3a814312 100644 --- a/fhirpath/src/main/java/au/csiro/pathling/sql/boundary/DecimalBoundaryFunction.java +++ b/fhirpath/src/main/java/au/csiro/pathling/sql/boundary/DecimalBoundaryFunction.java @@ -1,8 +1,9 @@ package au.csiro.pathling.sql.boundary; import java.math.BigDecimal; +import java.math.MathContext; import java.math.RoundingMode; -import java.util.Objects; +import java.util.Optional; import javax.annotation.Nullable; /** @@ -28,43 +29,66 @@ protected static BigDecimal highBoundaryForDecimal(@Nullable final BigDecimal d, private static BigDecimal calculateBoundary(@Nullable final BigDecimal d, @Nullable final Integer precision, final boolean isHigh) { // Check for null or invalid precision. - if (d == null || (precision != null && precision < 0)) { + final boolean precisionInvalid = + precision != null && (precision <= 0 || precision > MAX_PRECISION); + if (d == null || precisionInvalid) { return null; } - // Calculate the maximum scale that will fit within the maximum decimal. - final int integerLength = d.precision() - d.scale(); - final int maxScale = MAX_PRECISION - integerLength; - if (precision != null && precision > maxScale) { - return null; - } - - // Determine the digits that will potentially need to be added to the number based on whether - // it is negative and whether we are calculating a boundary that is further or closer to zero. final boolean inputIsNegative = d.compareTo(BigDecimal.ZERO) < 0; final boolean farBoundaryFromZero = isHigh ^ inputIsNegative; - final String digit = farBoundaryFromZero - ? "9" - : "0"; - - BigDecimal result = d; - if (farBoundaryFromZero) { - // Add the necessary number of extra digits to the decimal. - final int additionalDigits = Objects.requireNonNullElse(precision, maxScale) - d.scale(); - if (additionalDigits > 0) { - result = new BigDecimal(d.toPlainString() + (d.scale() == 0 - ? "." - : "") + digit.repeat(additionalDigits)); - } - } + final int significantFigures = Optional.ofNullable(precision).orElse(MAX_PRECISION); - // Determine the correct rounding mode based on whether we are calculating a high or low - // boundary. + // Round the number to the requested number of significant figures. final RoundingMode roundingMode = isHigh ? RoundingMode.CEILING : RoundingMode.FLOOR; - // Round the result to the desired precision. - return result.setScale(Objects.requireNonNullElse(precision, maxScale), roundingMode); + final BigDecimal rounded = d.round(new MathContext(significantFigures, roundingMode)); + + // Unscale the number to remove trailing zeroes. + final BigDecimal unscaled = new BigDecimal(rounded.stripTrailingZeros().unscaledValue()); + + // Expand the number to the far boundary if necessary. + final BigDecimal expanded = farBoundaryFromZero + ? inputIsNegative + ? unscaled.subtract(almostOne()) + : unscaled.add(almostOne()) + : unscaled; + + // Scale the number back to the original scale. + final int power = rounded.stripTrailingZeros().scale(); + final RoundingMode divisionRoundingMode = inputIsNegative ^ power < 0 + ? RoundingMode.CEILING + : RoundingMode.FLOOR; + final BigDecimal result = expanded.divide(pow10(power, significantFigures), + new MathContext(significantFigures, divisionRoundingMode)); + + // Convert the result back into plain notation. + return new BigDecimal(result.toPlainString()); + } + + private static BigDecimal almostOne() { + // Calculate the closest number to 1 that is less than 1. + final BigDecimal one = BigDecimal.ONE; + final MathContext mc = new MathContext(MAX_PRECISION); + return one.subtract(BigDecimal.valueOf(1, MAX_PRECISION), mc); + } + + private static BigDecimal pow10(final int exponent, final int precision) { + if (exponent == 0) { + return BigDecimal.ONE; // Any number to the power of 0 is 1. + } + + final int absExponent = Math.abs(exponent); + final BigDecimal positivePower = BigDecimal.TEN.pow(absExponent); + + if (exponent > 0) { + return positivePower; + } else { + // Calculate reciprocal for negative exponent. + final MathContext mc = new MathContext(precision, RoundingMode.HALF_UP); + return BigDecimal.ONE.divide(positivePower, mc); + } } } diff --git a/fhirpath/src/test/java/au/csiro/pathling/sql/boundary/DecimalBoundaryFunctionTest.java b/fhirpath/src/test/java/au/csiro/pathling/sql/boundary/DecimalBoundaryFunctionTest.java index 48b33727e0..335c83439a 100644 --- a/fhirpath/src/test/java/au/csiro/pathling/sql/boundary/DecimalBoundaryFunctionTest.java +++ b/fhirpath/src/test/java/au/csiro/pathling/sql/boundary/DecimalBoundaryFunctionTest.java @@ -16,34 +16,34 @@ class DecimalBoundaryFunctionTest { @Test @Order(1) - void lowBoundaryMaxPrecision() throws Exception { + void lowBoundaryMaxPrecision() { final BigDecimal d = new BigDecimal("1.587"); final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, null); log.debug("{}.lowBoundary() // {}", d, result); - assertEquals(new BigDecimal("1.5870000000000000000000000000000000000"), result); + assertEquals(new BigDecimal("1.587"), result); } @Test @Order(2) - void lowBoundaryContractedPrecision() throws Exception { + void lowBoundaryContractedPrecision() { final BigDecimal d = new BigDecimal("1.587"); - final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 2); - log.debug("{}.lowBoundary(2) // {}", d, result); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 3); + log.debug("{}.lowBoundary(3) // {}", d, result); assertEquals(new BigDecimal("1.58"), result); } @Test @Order(3) - void lowBoundaryExpandedPrecision() throws Exception { + void lowBoundaryExpandedPrecision() { final BigDecimal d = new BigDecimal("1.587"); - final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 4); - log.debug("{}.lowBoundary(4) // {}", d, result); - assertEquals(new BigDecimal("1.5870"), result); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 5); + log.debug("{}.lowBoundary(5) // {}", d, result); + assertEquals(new BigDecimal("1.587"), result); } @Test @Order(4) - void negativePrecision() throws Exception { + void negativePrecision() { final BigDecimal d = new BigDecimal("1.587"); final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, -1); log.debug("{}.lowBoundary(-1) // {}", d, result); @@ -52,7 +52,7 @@ void negativePrecision() throws Exception { @Test @Order(5) - void lowBoundaryNegativeMaxPrecision() throws Exception { + void lowBoundaryNegativeMaxPrecision() { final BigDecimal d = new BigDecimal("-1.587"); final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, null); log.debug("{}.lowBoundary() // {}", d, result); @@ -61,61 +61,61 @@ void lowBoundaryNegativeMaxPrecision() throws Exception { @Test @Order(6) - void lowBoundaryNegativeContractedPrecision() throws Exception { + void lowBoundaryNegativeContractedPrecision() { final BigDecimal d = new BigDecimal("-1.587"); - final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 2); - log.debug("{}.lowBoundary(2) // {}", d, result); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 3); + log.debug("{}.lowBoundary(3) // {}", d, result); assertEquals(new BigDecimal("-1.59"), result); } @Test @Order(7) - void lowBoundaryNegativeExpandedPrecision() throws Exception { + void lowBoundaryNegativeExpandedPrecision() { final BigDecimal d = new BigDecimal("-1.587"); - final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 4); - log.debug("{}.lowBoundary(4) // {}", d, result); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 5); + log.debug("{}.lowBoundary(5) // {}", d, result); assertEquals(new BigDecimal("-1.5879"), result); } @Test @Order(8) - void precisionHigherThanMax() throws Exception { + void precisionHigherThanMax() { final BigDecimal d = new BigDecimal("1.587"); - final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 38); - log.debug("{}.lowBoundary(38) // {}", d, result); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 39); + log.debug("{}.lowBoundary(39) // {}", d, result); assertNull(result); } @Test @Order(9) - void lowBoundaryIntegerMaxPrecision() throws Exception { + void lowBoundaryIntegerMaxPrecision() { final BigDecimal d = new BigDecimal("1"); final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, null); log.debug("{}.lowBoundary() // {}", d, result); - assertEquals(new BigDecimal("1.0000000000000000000000000000000000000"), result); + assertEquals(new BigDecimal("1"), result); } @Test @Order(10) - void lowBoundaryIntegerZeroPrecision() throws Exception { + void lowBoundaryIntegerZeroPrecision() { final BigDecimal d = new BigDecimal("1"); final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 0); log.debug("{}.lowBoundary(0) // {}", d, result); - assertEquals(new BigDecimal("1"), result); + assertNull(result); } @Test @Order(11) - void lowBoundaryIntegerExpandedPrecision() throws Exception { + void lowBoundaryIntegerExpandedPrecision() { final BigDecimal d = new BigDecimal("1"); - final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 4); - log.debug("{}.lowBoundary(4) // {}", d, result); - assertEquals(new BigDecimal("1.0000"), result); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 5); + log.debug("{}.lowBoundary(5) // {}", d, result); + assertEquals(new BigDecimal("1"), result); } @Test @Order(12) - void highBoundaryMaxPrecision() throws Exception { + void highBoundaryMaxPrecision() { final BigDecimal d = new BigDecimal("1.587"); final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, null); log.debug("{}.highBoundary() // {}", d, result); @@ -124,52 +124,52 @@ void highBoundaryMaxPrecision() throws Exception { @Test @Order(13) - void highBoundaryContractedPrecision() throws Exception { + void highBoundaryContractedPrecision() { final BigDecimal d = new BigDecimal("1.587"); - final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 2); - log.debug("{}.highBoundary(2) // {}", d, result); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 3); + log.debug("{}.highBoundary(3) // {}", d, result); assertEquals(new BigDecimal("1.59"), result); } @Test @Order(14) - void highBoundaryExpandedPrecision() throws Exception { + void highBoundaryExpandedPrecision() { final BigDecimal d = new BigDecimal("1.587"); - final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 4); - log.debug("{}.highBoundary(4) // {}", d, result); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 5); + log.debug("{}.highBoundary(5) // {}", d, result); assertEquals(new BigDecimal("1.5879"), result); } @Test @Order(15) - void highBoundaryNegativeMaxPrecision() throws Exception { + void highBoundaryNegativeMaxPrecision() { final BigDecimal d = new BigDecimal("-1.587"); final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, null); log.debug("{}.highBoundary() // {}", d, result); - assertEquals(new BigDecimal("-1.5870000000000000000000000000000000000"), result); + assertEquals(new BigDecimal("-1.587"), result); } @Test @Order(16) - void highBoundaryNegativeContractedPrecision() throws Exception { + void highBoundaryNegativeContractedPrecision() { final BigDecimal d = new BigDecimal("-1.587"); - final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 2); - log.debug("{}.highBoundary(2) // {}", d, result); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 3); + log.debug("{}.highBoundary(3) // {}", d, result); assertEquals(new BigDecimal("-1.58"), result); } @Test @Order(17) - void highBoundaryNegativeExpandedPrecision() throws Exception { + void highBoundaryNegativeExpandedPrecision() { final BigDecimal d = new BigDecimal("-1.587"); - final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 4); - log.debug("{}.highBoundary(4) // {}", d, result); - assertEquals(new BigDecimal("-1.5870"), result); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 5); + log.debug("{}.highBoundary(5) // {}", d, result); + assertEquals(new BigDecimal("-1.587"), result); } @Test @Order(18) - void highBoundaryIntegerMaxPrecision() throws Exception { + void highBoundaryIntegerMaxPrecision() { final BigDecimal d = new BigDecimal("1"); final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, null); log.debug("{}.highBoundary() // {}", d, result); @@ -178,20 +178,128 @@ void highBoundaryIntegerMaxPrecision() throws Exception { @Test @Order(19) - void highBoundaryIntegerZeroPrecision() throws Exception { + void highBoundaryIntegerZeroPrecision() { final BigDecimal d = new BigDecimal("1"); final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 0); log.debug("{}.highBoundary(0) // {}", d, result); - assertEquals(new BigDecimal("1"), result); + assertNull(result); } @Test @Order(20) - void highBoundaryIntegerExpandedPrecision() throws Exception { + void highBoundaryIntegerExpandedPrecision() { final BigDecimal d = new BigDecimal("1"); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 5); + log.debug("{}.highBoundary(5) // {}", d, result); + assertEquals(new BigDecimal("1.9999"), result); + } + + @Test + @Order(21) + void lowBoundaryTruncateFraction() { + final BigDecimal d = new BigDecimal("12.587"); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 2); + log.debug("{}.lowBoundary(2) // {}", d, result); + assertEquals(new BigDecimal("12"), result); + } + + @Test + @Order(22) + void highBoundaryTruncateFraction() { + final BigDecimal d = new BigDecimal("12.587"); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 2); + log.debug("{}.highBoundary(2) // {}", d, result); + assertEquals(new BigDecimal("13"), result); + } + + @Test + @Order(23) + void lowBoundaryTrailingZeroes() { + final BigDecimal d = new BigDecimal("12.500"); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 4); + log.debug("{}.lowBoundary(4) // {}", d, result); + assertEquals(new BigDecimal("12.5"), result); + } + + @Test + @Order(24) + void highBoundaryTrailingZeroes() { + final BigDecimal d = new BigDecimal("12.500"); final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 4); log.debug("{}.highBoundary(4) // {}", d, result); - assertEquals(new BigDecimal("1.9999"), result); + assertEquals(new BigDecimal("12.59"), result); + } + + @Test + @Order(25) + void lowBoundaryInsignificantOnLeft() { + final BigDecimal d = new BigDecimal("120"); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 2); + log.debug("{}.lowBoundary(2) // {}", d, result); + assertEquals(new BigDecimal("120"), result); + } + + @Test + @Order(26) + void highBoundaryInsignificantOnLeft() { + final BigDecimal d = new BigDecimal("120"); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 2); + log.debug("{}.highBoundary(2) // {}", d, result); + assertEquals(new BigDecimal("130"), result); + } + + @Test + @Order(27) + void lowBoundaryNegativeInsignificantOnLeft() { + final BigDecimal d = new BigDecimal("-120"); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 2); + log.debug("{}.lowBoundary(2) // {}", d, result); + assertEquals(new BigDecimal("-130"), result); + } + + @Test + @Order(28) + void highBoundaryNegativeInsignificantOnLeft() { + final BigDecimal d = new BigDecimal("-120"); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 2); + log.debug("{}.highBoundary(2) // {}", d, result); + assertEquals(new BigDecimal("-120"), result); + } + + @Test + @Order(29) + void lowBoundaryLessThanOne() { + final BigDecimal d = new BigDecimal("0.0034"); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 1); + log.debug("{}.lowBoundary(1) // {}", d, result); + assertEquals(new BigDecimal("0.003"), result); + } + + @Test + @Order(30) + void highBoundaryLessThanOne() { + final BigDecimal d = new BigDecimal("0.0034"); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 1); + log.debug("{}.highBoundary(1) // {}", d, result); + assertEquals(new BigDecimal("0.004"), result); + } + + @Test + @Order(31) + void lowBoundaryNegativeLessThanOne() { + final BigDecimal d = new BigDecimal("-0.0034"); + final BigDecimal result = DecimalBoundaryFunction.lowBoundaryForDecimal(d, 1); + log.debug("{}.lowBoundary(1) // {}", d, result); + assertEquals(new BigDecimal("-0.004"), result); + } + + @Test + @Order(32) + void highBoundaryNegativeLessThanOne() { + final BigDecimal d = new BigDecimal("-0.0034"); + final BigDecimal result = DecimalBoundaryFunction.highBoundaryForDecimal(d, 1); + log.debug("{}.highBoundary(1) // {}", d, result); + assertEquals(new BigDecimal("-0.003"), result); } }