Repository navigation
Do not answer an exhausted search with the empty set (#1036, #746 tier 4) - #1046
Merged
Merged
Conversation
…r 4)
`Solve` returned an empty `FiniteSet` both for an equation shown to have no roots
and for one every solver declined. The empty set is a positive claim, so the
second was a wrong answer: `x^6 + x*y + 1 = 0` has six roots for every `y` and
came back as `{ }`.
The two exits that mean "nothing settled this" now answer with the equation as a
set builder — the spelling `AnalyticalEquationSolver` already uses for #278 and
#964 — while an equation whose emptiness was established keeps `{ }`. Newton's
method is included: a search from finitely many starting points inside a bounded
region finding nothing is a fact about the search.
A conjunction with an unsettled side is answered as the conjunction. Intersecting
a finite set with a condition keeps the elements whose membership could not be
decided, so `x^6 + x*y + 1 = 0 and x - 1 = 0` would have become `{ 1 }` — one
false claim in place of another, since 1 solves the first only at `y = -2`.
Four theory rows recorded the old answer as correct. Each pinned an equation the
solver gives up on and that has roots, so they move to the new test file with the
honest assertion rather than being loosened.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd
Both sides only add to BREAKING-CHANGES.md's Unreleased section; the resolution keeps both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd
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.
Step 1 of #1036. Roadmap
#746 tier 4 — "
Solvestill returns anempty set both for 'no solutions' and for 'I gave up'." Does not close #1036, which has five steps.
What was wrong
SolvereturnsEntity.Set, and an emptyFiniteSetmeant two different things. Measured oncd0b56b9, the merge base:"x6 + x y + 1 = 0".ToEntity().Solve("x"){ }y"sin(x) + x + y = 0".ToEntity().Solve("x"){ }y"e^x + x + y = 0".ToEntity().Solve("x"){ }y"x6 + x y + 1".ToEntity().SolveEquation("x"){ }"e^x = 0".ToEntity().Solve("x"){ }"abs(x) = -1".ToEntity().Solve("x"){ }Six identical answers; the last two are true and the first four are not. The empty set is a positive
mathematical claim — no
xsatisfies this — so asserting it from a search that ran out of ideas isa wrong answer rather than a graceful failure.
The two sites
The issue names
Solvers.Definition.cs:346andAnalyticalEquationSolver.cs:337. Re-located on thistree:
Functions/Continuous/Solvers/EquationSolver/AnalyticalEquationSolver.cs:337—return Enumerable.Empty<Entity>().ToSet();, still at that line. This is the chokepoint: every solverbelow
Solveconverges here.Functions/Continuous/Solvers/EquationSolver/AnalyticalEquationSolver.cs:335— one line aboveit,
expr.SolveNt(x)returning nothing. Newton is asked from finitely many starting points insidea bounded region, so finding nothing is a fact about the search.
Solvers.Definition.cs:346is not inFunctions/Continuous/Solvers/— that file is 102 lineslong and holds
Entity.Solve. Line 346 is inFunctions/Continuous/Limits/Solvers/Solvers.Definition.cs,return MathS.NaN;at the end of thetwo-sided branch. It is a limit-pipeline site, not a
Solveone, and it is left for step 2 — seeWhat this deliberately does not do.
What it returns now
The equation itself, as the set of the
xthat satisfy it:With
AllowNewtonoff there is nothing numerical left either:And what was partly settled keeps the part that was — this one is the shape of the whole change,
because the exponential solver did settle the equation in
2 ^ (x sin(x))and it was the inversionof
x * sin(x)that had nothing to say:Why a
ConditionalSet, and what was rejectedChosen: the unsolved equation as a
ConditionalSet. It is already this codebase's spelling for"I did not settle this" in the solver itself —
#278 (
a = banswered{ x : a - b = 0 })and #1025/#964
(
UnsolvedWhereIndependenceIsDenied). It needs no new type, no new mechanism, and no change toSolve's return type. It is not merely a marker: it names the solution set, so a caller cansubstitute into it,
Filterit, unite it with what was found, or askTryContains— all of whichthis change exercises. And it composes: a product one of whose factors was solved comes back as
{ 1 } \/ { x : ... }, keeping the root that was found.Rejected: a
BudgetOutcome-carrying result (#1035's machinery, which merged today). That layeranswers which resource ran out where, and nothing here ran out of a resource — the solvers simply
have no method for these equations. Attaching a budget outcome to "no algorithm applies" would say
something false about why, and would put a second meaning into a type that has one. #1035 stays the
mechanism for the sites that really are resource limits, which is most of the remaining ~66.
Rejected: something on the side a caller opts into — a
TrySolveoverload, an out-parameter, anambient recording scope. All of them leave the default answer wrong, which is the whole complaint.
The rule at the top of
AGENTS.mdis that a published API returning the wrong answer is a bug withusers, not an asset to preserve.
Rejected: leaving the empty set and documenting it. It is not underspecified, it is false.
The conjunction, which had to be handled
Once an unsettled equation is a
ConditionalSet,andreachesSetOperators.IntersectFiniteSetAndSet, which keeps an element whose membership in the other operandcould not be decided:
That reading is older than this branch and reachable without it, but through
Solveit would haveturned one false claim into another —
x^6 + x*y + 1 = 0 and x - 1 = 0becoming{ 1 }, when 1solves the first only at
y = -2. SoStatementSolveranswers a conjunction with an unsettled sideas the conjunction:
Two settled sides still intersect (
(x - 3)(x - 6) = 0 and (x - 3)(x - 7) = 0is{ 3 }), adisjunction is still a union and keeps the root it had, and
impliesgets strictly more precise(
{ 1 } \/ BBbecomes{ 1 } \/ (BB \ { x : ... })). The underlyingIntersectFiniteSetAndSetreading is left alone and reported on #1036 — it is the same confusion ina shared set operator, and fixing it there is a change with repository-wide blast radius that wants
its own PR.
Public-API blast radius
No signature changes.
Solve,SolveEquationandMathS.SolveEquationhave always been typedSetand have always been able to return anInterval, aConditionalSetor a union; what changesis how often they do. What breaks is code that assumed a
FiniteSet:Checked and unaffected: the system solver (
EquationSolver.InSolveSystemalready testsis not FiniteSetand moves on —MathS.Equations("x + y - 3", "x - y - 1").Solve("x", "y")is[[2, 1]]on both builds, and so is the six-root system overx6 + x y + 1); the inequalitysolvers;
Entity.Solve'sInnerSimplified, sinceConditionalSet.InnerSimplifydoes not call backinto
Solve; the F# wrapper (solutionsreturnsEntity.Set, 134 tests pass); and all fourSampleNet5Solvelines, byte-identical on both builds.Two XML doc examples printed
{ }for an equation the solver gives up on and are corrected here(
Entity.Solve's own<example>, andMathS.Settings.AllowNewton's).BREAKING-CHANGES.mdcarries all of it underUnreleased, with three rows in the at-a-glance table.The tests, and the four that recorded the old answer
Sources/Tests/UnitTests/Algebra/SolveTest/UnsolvedEquationTest.cs, 25 tests. Built on the mergebase they fail 11 of 19 (before the conjunction ones were added), and the eight that pass there are
the controls: the impossible equations stay empty, the solvable ones stay solved, Newton still
answers.
Four theory rows asserted
rootCount: 0for equations the solver gives up on:SolveOneEquation.TestExponentialSolver("2 ^ (x sin(x)) + 4 ^ (x sin(x)) + c", 0)SolveOneEquation.FractionedPoly("x + sqrt(x^0.1 + a) + c", 0)SolveOneEquation.FractionedPoly("(x + 6)^(1/6) + x + x3 + a", 0)SolveOneEquation.FractionedPoly("sqrt(x + 1) + sqrt(x + 2) + a + x", 0)Each of these equations has roots for suitable parameter values, so
rootCount: 0was pinning thewrong answer. They move into the new file with the honest assertion — that the answer is the
condition and is not empty — rather than having their assertion loosened.
The new file takes the default
2019-2022header:Tests/UnitTests/Algebra/SolveTest/has nodirectory-wide scope in
Sources/.editorconfig, and that file has been a conflict magnet today, soit is left untouched.
Measured
Suite — 8091 passed, 0 failed, 14 skipped (8105 total) against 8070 / 0 / 14 (8084) on the merge
base: 25 added, 4 removed rows. F# wrapper 134 passed, 0 failed. Every value in the tables above
measured on a build of each side.
Cost — three arms in one process, each an
AssemblyLoadContextover its ownAngouriMath.dll:the merge base, this branch, and a byte-identical second copy of the merge base as the noise floor.
Ten equations that solve, three that do not, twenty repetitions, interleaved, third round reported;
three runs:
Read against the control, not against the merge base: two byte-identical builds differ by up to 370
KB of allocation and 8% of wall clock depending on load position, and this branch sits inside that
on both arms — it is faster than the control on one run of each and slower on the others. On the
give-up arm it allocates 4,672 bytes more than the control on 10.1 MB (+0.046%), which is the
ConditionalSetit now builds. The common case is untouched by construction: an equation thatsolves never reaches the changed lines.
The CI performance gate is safe — all five of
CommonFunctionsInterVersion'sSolve*benchmarksreturn the same
FiniteSeton both builds.What this deliberately does not do
InvertNode's empty returns, which produce an empty answer atAnalyticalEquationSolver.cs:157and:184and are genuinely mixed. Measured, three cases:abs(x) = -1is empty becauseInvertproduced a candidate carryingprovided -1 >= 0, which isa proof;
e^x = 0is empty becauseInvertproducedln(0)and dropped it as non-finite, also aproof;
x! - 6 = 0is empty becauseFactorialf.InvertNodereturns nothing at all, which is agive-up — and 3 is a root.
Modf,Summationf,Productf,LimitfandDerivativefdeclinethe same way, two of them with a comment saying so. Separating these means giving
InvertNodeaway to say "declined" across ~30 overrides, and it changes what
Invert's five callers read; itbelongs to Giving up on a resource is spelled the same as a mathematical negative #1036's step 5 rather than to this one, and
x! - 6 = 0is still{ }after this PR.Functions/Continuous/Limits/Solvers/Solvers.Definition.cs:346. Reached only where bothone-sided descents returned a value; every limit probed that reaches it (
1/x,abs(x)/x,sin(1/x)at 0) is one that genuinely does not exist, so nothing here demonstrates it live.Making it honest means separating a descent's
NaNfrom a proof throughout the limit pipeline,which is step 2's ledger.
SetOperators.IntersectFiniteSetAndSet, described above.🤖 Generated with Claude Code
https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd