JDK 14 RFR of JDK-8233452: java.math.BigDecimal.sqrt() with RoundingMode.FLOOR results in incorrect result

Brian Burkhalter brian.burkhalter at oracle.com
Tue Jan 14 21:59:29 UTC 2020


Hi Joe,

This looks good modulo a few picayune things I noticed in the implementation and test. Line numbers refer to the updated version of each file.

1. Implementation

2: Newer copyright year 2020, of course.
2150: s/much many/many/.
2185: I suppose that this is in case targetPrecision overflowed.
2231: Why ulp = ulp?
2376: s/larger than/larger/

2. Test

2: Newer copyright year 2020, of course.
39: Perhaps for symmetry with BigInteger (https://bugs.openjdk.java.net/browse/JDK-8152237) we should add BigDecimal.TWO at a later date.
112: s/valueOf(2)/TWO/
117: Alignment of RoundingMode.DOWN
137-138:

Probe inputs with two digits of precision, 10 … 99 and those
values scaled by 10^-1, 1.0, … 9.9, and by 10^-2, 0.1, … 0.99.

167: Two spaces after return
282, 303: “doesn’t not improperly” sounds awkward: should be “does not improperly”?
289, 310: “Sqrt root” also sounds awkward
296, 317: Extra space before “:”
316: s/down/up/ ?
337: s/rounding down/half even/ ?
362: s/near 1 half even/“near 1 “ + mc.toString()/ ?
397: delete commented out line?
520: s/+1)/+ 1)/
607: Introduce a square() method in BigSquareRoot also for parity with BigDecimal?

Brian

> On Jan 8, 2020, at 4:33 PM, Joe Darcy <joe.darcy at oracle.com> wrote:
> 
> Some further refinements to the implementation and tests ready for re-review:
> 
>     http://cr.openjdk.java.net/~darcy/8233452.4/ <http://cr.openjdk.java.net/~darcy/8233452.4/>
> The fix-up code for directed roundings (up, down, etc.) was corrected to properly handle rounding down when the interim result is a power of 10, in this case 1.0. The adjustment down in that case needs to be reduced in size since the size of an ulp changes at exponent boundaries. The regression tests cover this situation.
> 
> The assertion checks on the numerical properties of the result were restructured to be more informative. One assert was overly strict and made weaker to accommodate the sort of situation discussed in the comments.
> 
> The comments make reference to the "2p + 2" property. This concerns floating-point rounding and when a double-rounding problem can be avoided. In brief, if you first round to (p + k) digits than then round that result to p digits, a difference result can be computed than if a single rounding to p digits occurred. For example, both the roundings to (p + k) and p digits could round up when a single rounding up would not round up. However, if the first rounding is to at least (2p + 2) digits, a second rounding to p digits will *not* have the double rounding hazard for +, -, *, / and square root.
> 
> The main Newton loop in the square root implementation has been modified to compute at least (2p + 2) digits so the rounding to p digits will be correct without a fix-up. With sufficient analysis, computing to about p digitis instead and doing a fix-up should be possible, but I'll leave that as a refinement for another day.
> 



More information about the core-libs-dev mailing list