Skip to content

Stop presenting a near-rational root as an exact one (#235) - #668

Merged
Happypig375 merged 2 commits into
ASC-Community:masterfrom
Rafael-SOWNet:fix/rational-root-downcast
Aug 3, 2026
Merged

Happypig375 merged 2 commits into
ASC-Community:masterfrom
Rafael-SOWNet:fix/rational-root-downcast

Conversation

@Rafael-SOWNet

Copy link
Copy Markdown
Member

Closes #235.

"x + x + x + x + x + x + x41 + 1 = 0".Solve("x")   →  { -1/6 }

-1/6 is not a root. Substituted back it leaves
-1/80204967233062404407033075859456, which the reporter pointed out three years ago.

Why it was accepted

A numeric root that lands near a simple ratio is rewritten as that ratio, and that is
worth doing — Newton returns 0.4999999999999999 where 1/2 is meant. TryDowncast
widens PrecisionErrorZeroRange to 1e-7 so that FindRational will reach the ratio
through the noise.

But the same widened tolerance was then deciding whether the guess was right:

using var __ = MathS.Settings.PrecisionErrorZeroRange.Set(1e-7m);
var downcasted = Complex.Create(preciseValue.RealPart, preciseValue.ImaginaryPart);
if (equation.Substitute(x, downcasted).Evaled is not Complex error)
    return root;
return IsZero(error) && ...            // 1.25e-32 < 1e-7, so: yes

A residual of 1.25e-32 passes for zero at 1e-7, so a value that is merely close to a
root came back as though it were exact.

The change

Guessing the ratio and checking the guess are now separate questions with separate
answers. Where the residual comes out as an exact ratio — which it does whenever the
equation and the candidate are both rational, as here — it has to be exactly zero.
Where it does not, an equation carrying pi for instance, there is nothing to be exact
about and the ordinary tolerance still decides.

Tightening the 1e-7 itself was the other option, and it is the wrong one: that number
is doing a real job in reconstructing the ratio from a root Newton only knows to about
1e-15, and lowering it would lose the genuine downcasts this is meant to keep.

Result

The equation now answers its numeric root, which is honest about what is actually
known. What must not change, and does not:

2x - 1 = 0 1/2
x² - 1/4 = 0 ±1/2
3x + 2 = 0 -2/3
x² + 2x + 1 = 0 -1
x² - 2 = 0 ±sqrt(2), not rounded into a ratio

All five are in the tests, alongside the reported case and an assertion that the ratio
it used to return really is not a root.

Testing

UnitTests 3665 passed / 0 failed. No existing test needed changing, which is worth
saying explicitly for a change to how roots are presented.

Independent of #663, #664, #665, #666 and #667; touches no file any of them touch.

🤖 Generated with Claude Code

`x^41 + 6x + 1 = 0` answered { -1/6 }. That is not a root: it leaves
-1/80204967233062404407033075859456.

A numeric root that lands near a simple ratio is rewritten as that ratio, which is
worth doing -- Newton returns 0.4999999999999999 where 1/2 is meant. But the loose
tolerance that guesses the ratio was also deciding whether the guess was right, and
at 1e-7 a residual of 1.25e-32 passes for zero. So the guess was accepted, and a
value that is merely close to a root was returned as though it were exact.

Where the residual comes out as an exact ratio -- which it does whenever the equation
and the candidate are both rational, as here -- it now has to be exactly zero. Where
it does not, an equation carrying pi for instance, there is nothing to be exact about
and the ordinary tolerance still decides.

The equation now answers its numeric root instead, which is honest about what is
known. Roots that genuinely are ratios still come back as ratios, and irrational ones
are still not rounded into ratios; both are in the tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Aug 3, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.67%. Comparing base (90c00a8) to head (3818510).
⚠️ Report is 44 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #668      +/-   ##
==========================================
- Coverage   80.99%   79.67%   -1.32%     
==========================================
  Files         155      155              
  Lines       13687    12434    -1253     
  Branches     1957     2014      +57     
==========================================
- Hits        11086     9907    -1179     
+ Misses       1990     1935      -55     
+ Partials      611      592      -19     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Happypig375
Happypig375 merged commit 0fdec9d into ASC-Community:master Aug 3, 2026
21 of 24 checks passed
@Rafael-SOWNet
Rafael-SOWNet deleted the fix/rational-root-downcast branch August 4, 2026 20:49
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.

Wrong output due to a lost of precision.

3 participants