Skip to content

Root the reciprocal in integers for a negative-degree RootN [patch] - #161

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/rootn-negative-degree-142
Oct 6, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/rootn-negative-degree-142

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #142

Before: RootN(x, -n, d) was DivideApproximation(One, RootN(x, n, d + 2), d + 2, d). That rounds three times, and two guard digits can't absorb two of those roundings near a boundary. So RootN(3, -3, 10) returned 0.6933612743, where the correct answer is …744. The issue's sweep found 27 of 3,588 cases wrong, including some at 50 digits.

After: the negative path takes the root of the reciprocal directly in integers, as the issue's preferred fix describes:

  • The new PositiveReciprocalRootN computes q = floor(10^k / s). k is chosen so that q has at least n·(d+2) digits and n divides the exponent -k - e.
  • It then takes IntegerRootN(q, n). Since floor(floor(y)^(1/n)) = floor(y^(1/n)), the root is the true value truncated, and one ReduceSignificance(d) rounds it correctly.
  • The result is treated as exact only when 10^k % s == 0 and root^n == q. That matches how the positive path treats exact roots.
  • Negative x keeps the old behaviour: an even degree throws ArgumentOutOfRangeException, and an odd degree carries the sign. Zero still throws DivideByZeroException. A degree of -1 uses q directly, without a root.

Tests:

  • TestNegativeDegreeRootRoundsOnceFromTheTrueValue pins the issue's table: RootN(3, -3, 10) == 0.6933612744, RootN(20, -2, 10) == 0.2236067977, plus (47, -2, 5), (41, -3, 5), (0.5, -3, 8), (1.5, -3, 6), and a negative odd case.
  • TestNegativeDegreeRootsAgreeWithAWideReciprocalAcrossASweep covers the issue's differential sweep: k = 2..300, n ∈ {-2, -3, -5}, d ∈ {5, 10, 20, 50}. It compares each result with 1 / RootN(k, n, d+40) rounded to d. That reference goes through the positive path and Divide, so it shares no code with the path under test.
  • On main, both tests fail (RootN(3, -3, 10) and RootN(47, -2, 5) are wrong). With this change both pass. Full suite: 442 passed, 0 failed.

This doesn't touch #127 (FractionalPow double rounding). That one goes through exp(y·ln x), so the integer-root approach doesn't carry over directly.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VL7qukbkUpkVRArT45bBFb


Generated by Claude Code

RootN(x, -n, d) took the root at d+2 digits, its reciprocal at d+2, and
rounded again to d, so a value near a rounding boundary came out one
unit wrong in the last place (RootN(3, -3, 10) gave 0.6933612743). It
now takes the integer root of floor(10^k / s) directly: the floor of the
root of a floor is the floor of the root, so the digits are the true
value truncated and the single rounding to d is correct.

Fixes #142

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VL7qukbkUpkVRArT45bBFb
@sonarqubecloud

sonarqubecloud Bot commented Oct 6, 2026

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit 8f7c2e1 into main Oct 6, 2026
14 checks passed
@matt-edmondson
matt-edmondson deleted the fix/rootn-negative-degree-142 branch October 6, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RootN with a negative degree misrounds the last digit: RootN(3, -3, 10) returns 0.6933612743, correct is 0.6933612744

2 participants