Repository navigation
Stop CreateUnique from handing back a name already in use, fixing wrong integrals - #670
Merged
Happypig375 merged 2 commits intoAug 3, 2026
Conversation
`integral(x * (x^2 + 1)^2, x)` returned an answer whose derivative is not the
integrand: 10.16 against 9.41 at x = 1.3.
Substitution asks CreateUnique for a variable that does not appear in the
expression. CreateUnique looked for names to avoid by calling SplitIndex, which
cuts at the *first* underscore -- so for the prefix "u_sub", which is the one
integration uses, it read the existing u_sub_1 as the prefix "u" and the index
"sub_1", parsed no number from it, recorded nothing, and returned u_sub_1 as though
it were free.
That only bites when integration substitutes twice, because the second call is the
first one whose variable is itself a u_sub. Substituting u_sub_1^2 for u_sub_1 then
turned (u^2/2 + u) / (2u) into 3/4 -- a constant, containing no trace of the
variable being integrated, which is exactly the test for "the substitution worked".
So a nonsense expression was accepted and integrated.
CreateUnique now compares against the names in use rather than against indices
picked out of them, which does not depend on how SplitIndex parses anything.
Three tests change because the answers do. All three are now the closed forms, and
each was verified by differentiating back at several points:
int x(x^2+1)^3 dx (x^2 + 1)^4 / 8 was the expanded polynomial
int x(x^2+1)^10 dx (x^2 + 1)^11 / 22 was the expanded polynomial,
whose own comment noted it
differed from this by -1/22
int 3x^2(x^3+2)^2 dx (x^3 + 2)^3 / 3 was not an antiderivative at all
Two integrals that used to come back unevaluated, x(x^2+x+1)^2 and x^2(x^2+1)^2,
are solved now as well.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
x(x^2+x+1)^2 and x^2(x^2+1)^2 come back unevaluated on master and are solved by this change, but the fix that stops integration re-entering itself closes the path they reach an answer by, so with both applied they are unevaluated again. Asserting that they are solved makes the test brittle against a change that has nothing to do with variable naming. What matters is that no answer is a limit and a wrong answer is not, so only the latter is pinned. 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 #670 +/- ##
==========================================
- Coverage 80.99% 79.44% -1.56%
==========================================
Files 155 155
Lines 13687 12291 -1396
Branches 1957 1980 +23
==========================================
- Hits 11086 9764 -1322
+ Misses 1990 1941 -49
+ Partials 611 586 -25 ☔ 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.
A wrong answer in integration, and its cause is a variable-naming collision rather than
anything mathematical.
Not in the tracker. I found it by sweeping integrands numerically — differentiating each
answer back and comparing at points — after a related wrong answer turned up in a case
the test suite was asserting.
What happens
Substitution asks
Variable.CreateUniquefor a variable that does not occur in theexpression.
CreateUniqueworked out which names to avoid viaSplitIndex, which cutsat the first underscore. For the prefix
"u_sub"— the one integration uses — it readan existing
u_sub_1as prefix"u"and index"sub_1", parsed no number out of it,recorded nothing to avoid, and returned
u_sub_1.So the "fresh" variable was the variable being integrated. This only bites when
integration substitutes twice, because the second call is the first whose integration
variable is itself a
u_sub.The consequence is quiet. Substituting
u_sub_1^2 → u_sub_1turnsA constant — containing no trace of the variable being integrated, which is exactly the
test for "the substitution worked". A nonsense expression was accepted and integrated,
and the answer looked plausible.
The change
CreateUniquecompares against the names actually in use rather than against indicesparsed out of them, so it no longer depends on how
SplitIndexsplits anything. Anyprefix containing an underscore was affected;
"u_sub"is the one that reaches users.Result
Three existing tests change because the answers do. Each new answer was verified by
differentiating back at several points:
∫ x(x²+1)³ dx(x² + 1)⁴ / 8∫ x(x²+1)¹⁰ dx(x² + 1)¹¹ / 22∫ 3x²(x³+2)² dx(x³ + 2)³ / 3The second one is worth a look: its old expected value carried the comment
"differs from C + (x^2+1)^11 / 22 by -1/22". That is now simply the answer.
Two integrals that used to come back unevaluated —
x(x²+x+1)²andx²(x²+1)²— aresolved as well.
Testing
UnitTests3672 passed / 0 failed,FSharpWrapperUnitTests127 passed / 0 failed, andboth
netstandard2.0andnet7.0build.14 new tests: the two wrong answers, the two that were unsolved, the closed forms, and
the plain single-level substitutions that already worked and must keep working. They
assert by differentiating back rather than against printed strings, since pinning the
string is what let the original wrong answer sit in the suite.
Independent of #663 through #669.
🤖 Generated with Claude Code