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.da...@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. >
