Solve a polynomial through its factors rather than by its degree (#272) - #742
Merged
Merged
Conversation
The solver read a polynomial equation only as a polynomial: it gathered the monomials, took the degree, and applied the formula for that degree -- Cardano at three, Ferrari at four, nothing at all above four. A factorization was never looked for, and one handed over outright was expanded away before the degree was taken. So (x - 1)(x^2 - 3) = 0, whose roots are written on its face, came back as two nested cube roots of 26 + 18i, and a quintic that splits into a cubic's worth of whole roots and a quadratic fell through to the numeric solver. Two things are added. A product is now solved factor by factor, since a product is zero exactly where one of its factors is; and where the equation is written out as a sum, its rational roots are divided out first, which leaves a factor of lower degree for the formulas to answer exactly. Zeroing one factor only makes the product zero where the others are defined, so each root is checked against the whole product before it is kept -- x * (1/x) is undefined at 0, not zero there, and the verification the solvers share cannot catch that, since it drops a root only on positive evidence and a NaN residual is not evidence. Two-term polynomials are left alone. a*x^n + b is answered whole by inverting x^n = -b/a, which gives the roots as a + bi; splitting it off at its rational roots leaves the rest to be dug out of a quotient, and x^3 - 8 would read (-2 -+ sqrt(-12))/2 rather than (-1/2 +- i*sqrt(3)/2)*2. A test pins that. lim of the measured effect, against 40ce109: x^3 - x^2 - 3x + 3 = 0 -(-1 + (26 + 18i)^(1/3) + 10/(26 + 18i)^(1/3))/3, ... -> { 1, sqrt(3), -sqrt(3) } 160 ms -> 9 ms x^5 - x^4 - 7x^3 + 7x^2 + 12x - 12 = 0 { -2, -1.73205080756887741... - 2.3e-15i, 1, ... } -> { 1, 2, -2, sqrt(3), -sqrt(3) } 153 ms -> 12 ms (x - 1)(x^2 - 3) = 0 -> { 1, sqrt(3), -sqrt(3) } 138 ms -> 8 ms x^3 - 6x^2 + 11x - 6 = 0-> { 1, 2, 3 } 160 ms -> 1 ms 4929 unit tests and 130 F# tests pass; corpus 112/117 with 0 wrong, 0 error and 0 timeout, unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A sweep over 572 polynomials built from factors whose roots are known in advance
-- every product and every expansion of two or three factors drawn from a pool of
linear and irreducible quadratic ones -- says the fix does more than tidy the
output. Before it, ten of those came back **short of roots**, and after it none
do; no case gained a root that is not one, in either direction.
Every one of the ten had the same shape, a rational root times a quadratic times
x^2 + x + 1:
x^5 - 2x^3 - x^2 + 2 = 0 { 1, sqrt(2), -sqrt(2) }
-> all five, the pair (-1 +- i*sqrt(3))/2 included
Above degree four the equation went to the numeric solver whole, and that
searches the real axis, so the complex conjugate pair was simply absent -- and a
set two roots short reads exactly like a complete one. Splitting off the rational
root leaves a quartic, which is answered in one piece.
This is the stronger half of #272 and it was not visible from the issue, which
reports the answers as ugly rather than as incomplete.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 #272.
What was wrong
The solver read a polynomial equation only as a polynomial: gather the monomials, take the degree, apply the formula for that degree — Cardano at three, Ferrari at four, nothing at all above four. A factorization was never looked for, and one handed over outright was expanded away before the degree was taken.
The second factor is answered exactly when asked on its own. Handed to the solver as part of a product, it went through a cubic instead.
The half the issue does not mention: roots went missing
A sweep over 572 polynomials built from factors whose roots are known in advance (every product and every expansion of two or three factors drawn from a pool of linear and irreducible quadratics) found that ten of them came back short of roots:
Above degree four the equation went to the numeric solver whole, and that searches the real axis — so the complex conjugate pair was simply absent. A root set two roots short does not look any different from a complete one.
After the fix, 0 of 572 are short of roots, and no case in either direction returns a value that is not a root.
What changed
PolynomialFactoring.TryFactorcould not be reused as it stands: it offers a factorization only when the polynomial splits completely into whole linear factors, which is right for the simplifier (a partly factored sum is not a nicer way to write the same thing) and exactly wrong here.Soundness
Zeroing one factor makes the product zero only where the other factors are defined, so each root is checked against the whole product before it is kept.
x * (1/x)is undefined at 0, not zero there. The verification every solver shares cannot catch this — it drops a root only on positive evidence, and aNaNresidual is deliberately not evidence (SolveStatement.IsSpurious). Pinned by a test.Deliberately left alone
a*x^n + bis answered whole by invertingx^n = -b/a, which gives the roots asa + bi. Splitting it at its rational roots leaves the rest to be dug out of a quotient:x^3 - 8 = 02, (-1/2 ± i·sqrt(3)/2)·22, (-2 ∓ sqrt(-12))/2Nothing is gained, so two-term polynomials are excluded, and a test pins that decision. The remaining ugliness there —
x^6 - 1answers a root as1/2 + i·sin(5/3·π)— is filed separately as #743 rather than folded into this.Measured
Against
40ce1093:x^3 - x^2 - 3x + 3 = 0-(-1 + (26+18i)^(1/3) + 10/(26+18i)^(1/3))/3, …{ 1, sqrt(3), -sqrt(3) }x^5 - x^4 - 7x^3 + 7x^2 + 12x - 12 = 0{ -2, -1.7320508075688774… - 2.3e-15i, 1, … }{ 1, 2, -2, sqrt(3), -sqrt(3) }x^5 - 2x^3 - x^2 + 2 = 0{ 1, sqrt(2), -sqrt(2) }— two roots missing(x - 1)(x^2 - 3) = 0{ 1, sqrt(3), -sqrt(3) }x^3 - 6x^2 + 11x - 6 = 0{ 1, 3, 2 }{ 1, 2, 3 }4931 unit tests and 130 F# tests pass. The 117-problem corpus stays at 112 solved with 0 wrong, 0 error, 0 timeout, and gets slightly faster overall (12.08 s → 11.26 s), so the extra factoring probe costs nothing measurable on non-polynomial equations.
🤖 Generated with Claude Code