This is an automated email from the ASF dual-hosted git repository.
erans pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-numbers.git
commit 7c20513b8cf82e0e485c1a953f9cb95f0f277908
Author: Schamschi <heinrich.bo...@gmx.at>
AuthorDate: Mon Jun 24 22:58:58 2019 +0200
NUMBERS-120: Repair BigFraction.doubleValue() to produce correctly rounded
result with maximum precision
---
.../commons/numbers/fraction/BigFraction.java | 155 +++++++++++++++++++--
.../commons/numbers/fraction/BigFractionTest.java | 53 ++++++-
2 files changed, 189 insertions(+), 19 deletions(-)
diff --git
a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
index 7c8ce9c..d9efcaf 100644
---
a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
+++
b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
@@ -671,20 +671,149 @@ public class BigFraction extends Number implements
Comparable<BigFraction>, Seri
*/
@Override
public double doubleValue() {
- double doubleNum = numerator.doubleValue();
- double doubleDen = denominator.doubleValue();
- double result = doubleNum / doubleDen;
- if (Double.isInfinite(doubleNum) ||
- Double.isInfinite(doubleDen) ||
- Double.isNaN(result)) {
- // Numerator and/or denominator must be out of range:
- // Calculate how far to shift them to put them in range.
- int shift = Math.max(numerator.bitLength(),
- denominator.bitLength()) -
Math.getExponent(Double.MAX_VALUE);
- result = numerator.shiftRight(shift).doubleValue() /
- denominator.shiftRight(shift).doubleValue();
+ if (numerator.signum() == 0) {
+ return 0;
+ }
+
+ /*
+ * We will manually construct a long from which to create the double to
+ * ensure maximum precision. First, we set the sign:
+ */
+ long sign = numerator.signum() == -1 ? 1L : 0L;
+ BigInteger positiveNumerator = numerator.abs();
+
+ /*
+ * The significand of a double value consists of 52 bits for the
+ * fractional part, plus the implicit leading 1 bit for the
+ * non-fractional part, which makes 53 bits in total. So we need to
+ * calculate the most significant 54 bits of this fraction's quotient,
+ * and then, based on the 54th most significant bit, find out whether
+ * we need to round up or down.
+ *
+ * First, we'll remove all powers of 2 from the denominator because
they
+ * are not relevant for the significand of the prospective double
value.
+ */
+ int denRightShift = denominator.getLowestSetBit();
+ BigInteger divisor = denominator.shiftRight(denRightShift);
+
+ /*
+ * Now, we're going to calculate the 54 most significant bits of the
+ * quotient using integer division. To guarantee that the quotient has
+ * at least 54 bits, the bit length of the dividend must exceed that
+ * of the divisor by at least 54. If the numerator has powers of 2
+ * beyond this limit, they can be removed as well.
+ */
+ int numRightShift = positiveNumerator.bitLength() -
divisor.bitLength() - 54;
+ if (numRightShift > 0) {
+ numRightShift = Math.min(numRightShift,
positiveNumerator.getLowestSetBit());
+ }
+ BigInteger dividend = positiveNumerator.shiftRight(numRightShift);
+
+ BigInteger quotient = dividend.divide(divisor);
+ int quotRightShift = quotient.bitLength() - 53;
+
+ /*
+ * If the denominator was a power of two, then the divisor was reduced
+ * 1, meaning the quotient was calculated exactly. Otherwise, the
+ * fractional part of the precise quotient's binary representation does
+ * not terminate.
+ */
+ long significand = roundAndRightShift(
+ quotient,
+ quotRightShift,
+ !divisor.equals(BigInteger.ONE)
+ ).longValue();
+
+ /*
+ * If the significand had to be rounded up, this could have caused an
+ * overflow into the 54th least significant bit.
+ */
+ if ((significand & (1L << 53)) != 0) {
+ significand >>= 1;
+ quotRightShift++;
+ }
+
+ /*
+ * Now comes the exponent. The absolute value of this fraction based
+ * on the current local variables is:
+ *
+ * significand * 2^(numRightShift - denRightShift + quotRightShift)
+ *
+ * To get the unbiased exponent for the double value, we need to add 52
+ * to the above exponent, because the 52 least significant bits of
+ * significant will be treated as a fractional part. To convert the
+ * unbiased exponent to a biased exponent, we need to add 1023.
+ */
+ long exponent = numRightShift - denRightShift + quotRightShift + 52 +
1023;
+
+ if (exponent > 2046) { //infinity
+ exponent = 2047;
+ significand = 0;
+ } else if (exponent > 0) { //normalized number
+ significand &= 0x000FFFFFFFFFFFFFL; //remove implicit leading 1-bit
+ } else { //smaller than the smallest normalized number
+ /*
+ * We need to round the quotient to fewer than 53 bits. This must
+ * be done with the original quotient and not with the current
+ * significand, because the loss of precision in the rounding to 53
+ * bits might cause a rounding of the current significand's value
+ * to produce a different result than a rounding of the
+ * original quotient.
+ *
+ * So we find out how many high-order bits from the quotient we can
+ * squeeze into the significand. The absolute value of the fraction
+ * is:
+ *
+ * quotient * 2^(numRightShift - denRightShift)
+ *
+ * To get the significand, we need to right shift the quotient so
+ * that the above exponent becomes (- 1022 - 52) = -1074 (-1022 is
+ * the unbiased exponent of a subnormal double value, and 52
because
+ * the significand will be treated as the fractional part)
+ */
+ significand = roundAndRightShift(
+ quotient,
+ - 1074 - (numRightShift - denRightShift),
+ !divisor.equals(BigInteger.ONE)
+ ).longValue();
+ exponent = 0L;
+ }
+ return Double.longBitsToDouble((sign << 63) | (exponent << 52) |
significand);
+ }
+
+ /**
+ * Rounds an integer to the specified power of two (i.e. the minimum
number of
+ * low-order bits that must be zero) and performs a right-shift by this
+ * amount. The rounding mode applied is round to nearest, with ties
rounding
+ * to even (meaning the prospective least significant bit must be zero).
The
+ * number can optionally be treated as though it contained at
+ * least one 0-bit and one 1-bit in its fractional part, to influence the
result in cases
+ * that would otherwise be a tie.
+ * @param value the number to round and right-shift
+ * @param bits the power of two to which to round; must be positive
+ * @param hasFractionalBits whether the number should be treated as though
+ * it contained a non-zero fractional part
+ * @return a {@code BigInteger} as described above
+ */
+ private static BigInteger roundAndRightShift(BigInteger value, int bits,
boolean hasFractionalBits) {
+ if (bits <= 0) {
+ throw new IllegalArgumentException();
+ } else if (bits > value.bitLength()) {
+ return BigInteger.ZERO;
+ } else {
+ boolean roundUp = false;
+ if (value.testBit(bits - 1)
+ && (hasFractionalBits
+ || (value.getLowestSetBit() < bits - 1)
+ || value.testBit(bits))) {
+ roundUp = true;
+ }
+ BigInteger result = value.shiftRight(bits);
+ if (roundUp) {
+ result = result.add(BigInteger.ONE);
+ }
+ return result;
}
- return result;
}
/**
diff --git
a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
index 74aefc4..7c4afdf 100644
---
a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
+++
b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/BigFractionTest.java
@@ -178,6 +178,35 @@ public class BigFractionTest {
Assertions.assertEquals(1.0 / 3.0, second.doubleValue(), 0.0);
}
+ @Test
+ public void testDoubleValueForSubnormalNumbers() {
+ {
+ double min = Double.MIN_VALUE;
+ double min1Up = Math.nextUp(min);
+ double min2Up = Math.nextUp(min1Up);
+ Assertions.assertEquals(
+ min,
+ BigFraction.from(min).doubleValue());
+ Assertions.assertEquals(
+ min1Up,
+ BigFraction.from(min1Up).doubleValue());
+ Assertions.assertEquals(
+ min2Up,
+ BigFraction.from(min2Up).doubleValue());
+ }
+
+ {
+ double minNormal1Down = Math.nextDown(Double.MIN_NORMAL);
+ double minNormal2Down = Math.nextDown(minNormal1Down);
+ Assertions.assertEquals(
+ minNormal1Down,
+ BigFraction.from(minNormal1Down).doubleValue());
+ Assertions.assertEquals(
+ minNormal2Down,
+ BigFraction.from(minNormal2Down).doubleValue());
+ }
+ }
+
// MATH-744
@Test
public void testDoubleValueForLargeNumeratorAndDenominator() {
@@ -202,15 +231,27 @@ public class BigFractionTest {
Assertions.assertEquals(5, large.floatValue(), 1e-15);
}
- // NUMBERS-15
@Test
public void testDoubleValueForLargeNumeratorAndSmallDenominator() {
- final BigInteger pow300 = BigInteger.TEN.pow(300);
- final BigInteger pow330 = BigInteger.TEN.pow(330);
- final BigFraction large = BigFraction.of(pow330.add(BigInteger.ONE),
- pow300);
+ // NUMBERS-15
+ {
+ final BigInteger pow300 = BigInteger.TEN.pow(300);
+ final BigInteger pow330 = BigInteger.TEN.pow(330);
+ final BigFraction large =
BigFraction.of(pow330.add(BigInteger.ONE),
+ pow300);
- Assertions.assertEquals(1e30, large.doubleValue(), 1e-15);
+ Assertions.assertEquals(1e30, large.doubleValue(), 1e-15);
+ }
+
+ // NUMBERS-120
+ {
+ BigFraction f = BigFraction.of(
+ BigInteger.ONE.shiftLeft(1024)
+ .subtract(BigInteger.ONE.shiftLeft(970))
+ .add(BigInteger.ONE),
+ BigInteger.valueOf(3));
+ Assertions.assertEquals(5.992310449541053E307, f.doubleValue());
+ }
}
// NUMBERS-15
