[9] RFR of 8032027: Add BigInteger square root methods
Louis Wasserman
wasserman.louis at gmail.com
Thu Dec 10 02:22:56 UTC 2015
Guava's tests check the explicit definition of square root (mentioned by
Joe above) on 2^n +/- 1 for all n up to Double.MAX_EXPONENT + 1, because
why not?
On Wed, Dec 9, 2015 at 6:12 PM Joseph D. Darcy <joe.darcy at oracle.com> wrote:
> Hi Brian,
>
> New version looks good.
>
> One more case to try: start with a BigInteger that would overflow to
> Double.POSITIVE_INFINITY when the doubleValue method was called. If this
> case doesn't take too long to run, it would be a fine additional case to
> add to the test. 2^1024 should be fine input value. More precisely,
>
> (new
> BigDecimal(Double.MAX_VALUE)).toBigInteger().add(BigInteger.ONE);
>
> should do the trick. If the code passes with this value, you're okay to
> push. Well, while you're at it, might as well verify
>
> (new BigDecimal(Double.MAX_VALUE)).toBigInteger()
>
> behaves well too ;-)
>
> Thanks,
>
> -Joe
>
> On 12/9/2015 5:41 PM, Brian Burkhalter wrote:
> > Hi Joe,
> >
> > On Dec 1, 2015, at 7:25 PM, Joseph D. Darcy <Joe.Darcy at Oracle.Com
> > <mailto:Joe.Darcy at Oracle.Com>> wrote:
> >
> >> Current version looks okay. One more request, before pushing please
> >> add explicit test cases for the for the largest number having 63 bits
> >> and the smallest number having 64 bits. No need for another round of
> >> webrevs for that.
> >
> > Well there is after all a need for another round of review:
> >
> > http://cr.openjdk.java.net/~bpb/8032027/webrev.04/
> > <http://cr.openjdk.java.net/%7Ebpb/8032027/webrev.04/>
> >
> > That was a good call to add the above tests: one of them failed. This
> > was found to be due to a floor() where there should have been a ceil().
> >
> > Summary:
> >
> > MutableBigInteger: at line 1920 change Math.floor(.) to Math.ceil(.).
> > BigIntegerTest: at lines 331-340 add testing of 2^N and 2^N - 1, 0 < N
> > < 1024
> >
> > Thanks,
> >
> > Brian
>
>
More information about the core-libs-dev
mailing list