Skip to content

Stop CreateUnique from handing back a name already in use, fixing wrong integrals - #670

Merged
Happypig375 merged 2 commits into
ASC-Community:masterfrom
Rafael-SOWNet:fix/unique-variable-collision
Aug 3, 2026
Merged

Happypig375 merged 2 commits into
ASC-Community:masterfrom
Rafael-SOWNet:fix/unique-variable-collision

Conversation

@Rafael-SOWNet

Copy link
Copy Markdown
Member

A wrong answer in integration, and its cause is a variable-naming collision rather than
anything mathematical.

∫ x·(x²+1)² dx     // at x = 1.3 the integrand is 9.4069
                   // the derivative of the answer is 10.1649

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.CreateUnique for a variable that does not occur in the
expression. CreateUnique worked out which names to avoid via SplitIndex, which cuts
at the first underscore. For the prefix "u_sub" — the one integration uses — it read
an existing u_sub_1 as 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_1 turns

(u²/2 + u) / (2u)      into      (3/2·u) / (2u)  =  3/4

A 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

CreateUnique compares against the names actually in use rather than against indices
parsed out of them, so it no longer depends on how SplitIndex splits anything. Any
prefix 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:

was now
∫ x(x²+1)³ dx the expanded polynomial (x² + 1)⁴ / 8
∫ x(x²+1)¹⁰ dx the expanded polynomial (x² + 1)¹¹ / 22
∫ 3x²(x³+2)² dx not an antiderivative (x³ + 2)³ / 3

The 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)² and x²(x²+1)² — are
solved as well.

Testing

UnitTests 3672 passed / 0 failed, FSharpWrapperUnitTests 127 passed / 0 failed, and
both netstandard2.0 and net7.0 build.

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

Rafael-SOWNet and others added 2 commits August 3, 2026 13:29
`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-commenter

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.44%. Comparing base (90c00a8) to head (8a2d90c).
⚠️ Report is 41 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     #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.
📢 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 d9e3dd0 into ASC-Community:master Aug 3, 2026
21 of 24 checks passed
@Rafael-SOWNet
Rafael-SOWNet deleted the fix/unique-variable-collision 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.

3 participants