Re: [9] RFR of 8032027: Add BigInteger square root methods

A new version is here: http://cr.openjdk.java.net/~bpb/8032027/webrev.05/ I changed the test to verify these values: Stream.Builder sb = Stream.builder(); int maxExponent = Double.MAX_EXPONENT + 1; for (int i = 1; i <= maxExponent; i++) { BigInteger p2 =

Re: [9] RFR of 8032027: Add BigInteger square root methods

Looks good Brian; thanks, -Joe On 12/10/2015 2:32 PM, Brian Burkhalter wrote: A new version is here: http://cr.openjdk.java.net/~bpb/8032027/webrev.05/ I changed the test to verify these values: Stream.Builder sb =

Re: [9] RFR of 8032027: Add BigInteger square root methods

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 wrote: > Hi Brian, > > New version looks good. > > One more case to

Re: [9] RFR of 8032027: Add BigInteger square root methods

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

Re: [9] RFR of 8032027: Add BigInteger square root methods

Hi Joe, On Dec 1, 2015, at 7:25 PM, Joseph D. Darcy 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

Re: [9] RFR of 8032027: Add BigInteger square root methods

Hi Joe, On Dec 1, 2015, at 7:25 PM, Joseph D. Darcy 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

Re: [9] RFR of 8032027: Add BigInteger square root methods

Hi Brian, On 11/30/2015 3:24 PM, Brian Burkhalter wrote: Hi Joe, On Nov 29, 2015, at 10:01 AM, joe darcy > wrote: The "if (...) " logic that is repeated a few times in this method could be pulled out into its own method, possibly one

Re: [9] RFR of 8032027: Add BigInteger square root methods

Hi Joe, On Nov 29, 2015, at 10:01 AM, joe darcy wrote: > The "if (...) " logic that is repeated a few times in this method could be > pulled out into its own method, possibly one structured a bit differently to > return the number of errors. > > I think it would be

Re: [9] RFR of 8032027: Add BigInteger square root methods

Hi Brian, On 11/25/2015 12:02 PM, Brian Burkhalter wrote: On Nov 16, 2015, at 10:00 AM, joe darcy wrote: Returning to this review… Likewise … Please refer to the updated patch at http://cr.openjdk.java.net/~bpb/8032027/webrev.02/. A few comments and suggestions:

Re: [9] RFR of 8032027: Add BigInteger square root methods

On Nov 16, 2015, at 10:00 AM, joe darcy wrote: > Returning to this review… Likewise … Please refer to the updated patch at http://cr.openjdk.java.net/~bpb/8032027/webrev.02/. > A few comments and suggestions: > > 2452 public BigInteger[] sqrtAndRemainder() { > 2453

Re: [9] RFR of 8032027: Add BigInteger square root methods

Returning to this review... On 10/7/2015 7:02 PM, Brian Burkhalter wrote: Joe / Andrew / Louis, Following up on the thread regarding https://bugs.openjdk.java.net/browse/JDK-8032027. I revised the algorithm similarly to what Louis suggested for the initial value of the iteration. I also

Re: [9] RFR of 8032027: Add BigInteger square root methods

Joe / Andrew / Louis, Following up on the thread regarding https://bugs.openjdk.java.net/browse/JDK-8032027. I revised the algorithm similarly to what Louis suggested for the initial value of the iteration. I also tightened up the memory usage. The updated version is here:

Re: [9] RFR of 8032027: Add BigInteger square root methods

Andrew / Joe, Thank you for the much appreciated comments. On Oct 3, 2015, at 11:45 AM, joe darcy wrote: > For an initial implementation, I think it is acceptable to use a simple > algorithm that is clearly correct. That algorithm can then be replaced with > faster ones

Re: [9] RFR of 8032027: Add BigInteger square root methods

On 02/10/15 21:41, Brian Burkhalter wrote: > Any suggestions as to the improvement of the approach concerning > memory use or any other aspect of the algorithm would be > appreciated, as would be suggestions regarding the test. This algorithm doesn't look like the best to me because it's got

Re: [9] RFR of 8032027: Add BigInteger square root methods

On 10/3/2015 2:38 AM, Andrew Haley wrote: On 02/10/15 21:41, Brian Burkhalter wrote: Any suggestions as to the improvement of the approach concerning memory use or any other aspect of the algorithm would be appreciated, as would be suggestions regarding the test. This algorithm doesn't look

Re: [9] RFR of 8032027: Add BigInteger square root methods

Here is a trick to consider for future performance tuning: all large floating-point numbers are integers. Once the size of the positive exponent exceeds the number of bits of precision, the value must be an integer. For double, that means values greater than 2^54 are integers and the full

[9] RFR of 8032027: Add BigInteger square root methods

Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8032027 Patch: http://cr.openjdk.java.net/~bpb/8032027/webrev.00/ Summary: Add sqrt() and sqrtAndRemainder() methods. The square root calculated is the integer square root ’s’ such that for a given integer ’n’

Re: [9] RFR of 8032027: Add BigInteger square root methods

I'm pretty sure we could potentially public-domain our implementation, if that were an issue. > The implementation I have here is effectively the one from Hacker’s Delight (2nd ed.). The initial guess is intended to be larger than the true result in order to simplify the termination condition of

Re: [9] RFR of 8032027: Add BigInteger square root methods

On Oct 2, 2015, at 2:16 PM, Louis Wasserman wrote: > I'm pretty sure we could potentially public-domain our implementation, if > that were an issue. Personally, if it’s much better than mine (or what mine could be revised to be) I’d be happy to have the better

Re: [9] RFR of 8032027: Add BigInteger square root methods

Have you investigated Guava's really quite tightly optimized implementation here? https://github.com/google/guava/blob/master/guava/src/com/google/common/math/BigIntegerMath.java#L242 A few years back I submitted a patch (now applied) to make BigInteger.doubleValue() very fast. If I recall

Re: [9] RFR of 8032027: Add BigInteger square root methods

I am aware of the classes at http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/math/BigIntegerMath.html but have not looked at anything beyond the javadoc specification primarily in the interest of licensing purity. Perhaps that is not a problem but I preferred to stay