Repository navigation
Stop presenting a near-rational root as an exact one (#235) - #668
Merged
Happypig375 merged 2 commits intoAug 3, 2026
Merged
Conversation
`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 Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #235.
-1/6is 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.4999999999999999where1/2is meant.TryDowncastwidens
PrecisionErrorZeroRangeto 1e-7 so thatFindRationalwill reach the ratiothrough the noise.
But the same widened tolerance was then deciding whether the guess was right:
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
pifor instance, there is nothing to be exactabout 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 = 01/2x² - 1/4 = 0±1/23x + 2 = 0-2/3x² + 2x + 1 = 0-1x² - 2 = 0±sqrt(2), not rounded into a ratioAll 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
UnitTests3665 passed / 0 failed. No existing test needed changing, which is worthsaying 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