<div dir="ltr">Apologies if this has been discussed before.<br><br>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.<div><br></div><div>AFAICT, the two options here are:</div><div><br></div><div>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.)</div><div><br></div><div>2) Produce an ArithmeticException with a justification along the lines of "The result would not be meaningful."</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>Hans</div></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Fri, Jul 11, 2025 at 7:42 AM fabioromano1 <<a href="mailto:duke@openjdk.org">duke@openjdk.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> This PR implements nth root computation for BigIntegers using Newton method.<br>
<br>
fabioromano1 has updated the pull request incrementally with one additional commit since the last revision:<br>
<br>
  Optimize the computation of the input's shift<br>
<br>
  Optimize the computation of the input's shift, in order to avoid having more non-significant bits than necessary in the initial estimate.<br>
<br>
-------------<br>
<br>
Changes:<br>
  - all: <a href="https://git.openjdk.org/jdk/pull/24898/files" rel="noreferrer" target="_blank">https://git.openjdk.org/jdk/pull/24898/files</a><br>
  - new: <a href="https://git.openjdk.org/jdk/pull/24898/files/788a82b9..6dcd6792" rel="noreferrer" target="_blank">https://git.openjdk.org/jdk/pull/24898/files/788a82b9..6dcd6792</a><br>
<br>
Webrevs:<br>
 - full: <a href="https://webrevs.openjdk.org/?repo=jdk&pr=24898&range=18" rel="noreferrer" target="_blank">https://webrevs.openjdk.org/?repo=jdk&pr=24898&range=18</a><br>
 - incr: <a href="https://webrevs.openjdk.org/?repo=jdk&pr=24898&range=17-18" rel="noreferrer" target="_blank">https://webrevs.openjdk.org/?repo=jdk&pr=24898&range=17-18</a><br>
<br>
  Stats: 18 lines in 1 file changed: 8 ins; 0 del; 10 mod<br>
  Patch: <a href="https://git.openjdk.org/jdk/pull/24898.diff" rel="noreferrer" target="_blank">https://git.openjdk.org/jdk/pull/24898.diff</a><br>
  Fetch: git fetch <a href="https://git.openjdk.org/jdk.git" rel="noreferrer" target="_blank">https://git.openjdk.org/jdk.git</a> pull/24898/head:pull/24898<br>
<br>
PR: <a href="https://git.openjdk.org/jdk/pull/24898" rel="noreferrer" target="_blank">https://git.openjdk.org/jdk/pull/24898</a><br>
</blockquote></div>