Apologies if this has been discussed before.
The explanation for the exception for negative n ("This would cause
the operation to yield a non-integer value.") feels wrong to me. I think
the definition of the result as: x.signum() * floor(abs(nthRoot(x, n)))
works fine for that case, if nthRoot(x,-n) is 1/(nthRoot(x,n)) as expected,
right? And the main definition of the result just isn't well-defined.
Neither yields a non-integer.
AFAICT, the two options here are:
1) Just produce the, usually zero, result, usually with a remainder of
`this`. (If 'this' is zero, you would still need an arithmetic exception,
and +-1 is also interesting.)
2) Produce an ArithmeticException with a justification along the lines of
"The result would not be meaningful."
Based on the Wikipedia page, negative n aren't normally a thing, so (2)
seems justifiable. But I think the current justification for the exception
is not.
I can't think of a good reason to prefer (1), but we've all been bitten by
unnecessary domain restrictions, so I'm not positive about that.
Hans
On Fri, Jul 11, 2025 at 7:42 AM fabioromano1 <d...@openjdk.org> wrote:
> > This PR implements nth root computation for BigIntegers using Newton
> method.
>
> fabioromano1 has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Optimize the computation of the input's shift
>
> Optimize the computation of the input's shift, in order to avoid having
> more non-significant bits than necessary in the initial estimate.
>
> -------------
>
> Changes:
> - all: https://git.openjdk.org/jdk/pull/24898/files
> - new: https://git.openjdk.org/jdk/pull/24898/files/788a82b9..6dcd6792
>
> Webrevs:
> - full: https://webrevs.openjdk.org/?repo=jdk&pr=24898&range=18
> - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24898&range=17-18
>
> Stats: 18 lines in 1 file changed: 8 ins; 0 del; 10 mod
> Patch: https://git.openjdk.org/jdk/pull/24898.diff
> Fetch: git fetch https://git.openjdk.org/jdk.git
> pull/24898/head:pull/24898
>
> PR: https://git.openjdk.org/jdk/pull/24898
>
