Skip to content

Simplify does not add a condition the input does not have - #2171

Merged
Rafael-SOWNet merged 1 commit into
masterfrom
a-division-in-placeholders-keeps-the-symbols-values
Oct 27, 2026
Merged

Rafael-SOWNet merged 1 commit into
masterfrom
a-division-in-placeholders-keeps-the-symbols-values

Conversation

@Rafael-SOWNet

@Rafael-SOWNet Rafael-SOWNet commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Part of #2134: its second row. The first row, a condition left out, is the question #1648 decides.

The defect: PolynomialLongDivision divides by the divisor's leading coefficient. Its first attempt, on the pair as written, declines a symbolic one for the simplifier, since a quotient that loses a value of a parameter is not an equivalence. The second attempt, with the variables replaced by temporaries, has no such guard. It predates the guard, and #1269 left it as it was.

(t x + y)/(1 - y x) divided there in y gives -1/x + (t x + 1/x)/(1 - x y). That is undefined at x = 0, where the divisor is 1, and Simplify passed the condition on:

Simplify 2.6.0 this PR
-(t*x + y)/(1 - y*x) + (t*x + y)/(1 - y*x) 0 provided not 1 - x * y = 0 and not x = 0 0 provided not 1 - x * y = 0

At x = 0 the input is -y + y, so the old answer was undefined at a point where the input is defined.

The change: the replaced attempt now also declines a symbolic leading coefficient, where the divisor has more than one term in the variable.

A divisor of one term, a x^k, is still divided. It is zero wherever its coefficient is, so dividing by that coefficient loses nothing the input had. That case is load-bearing: the limit of (tan(a x) - sin(b x))/(a x) at 0 reaches (a - b)/a through it. Without it the limit comes out 1 - b/a, because Simplify does not put 1 - b/a over one denominator (#2137).

Checked:

  • SimplifiedConditionNotStricterTest evaluates the answer at x = 0, y = 2, t = 3 and expects 0. The old answer gives NaN there.
  • Unit suite: 17,130 passed, 0 failed.
  • SimpSweep: 10,463/10,463 agree.
  • CanonCheck: matches its baseline.
  • BoundCheck: 0 disagreements.
  • CasBench: 120/121, unchanged.
  • BREAKING-CHANGES entry added, measured on the 2.6.0 package.
  • codecov/project is red at −0.02% (5 hits fewer). The new guard returns before lines that tests used to reach, so that is expected. The status fails on any decrease at all, which is a configuration problem rather than a finding, and is taken up separately.

🤖 Generated with Claude Code

https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura

PolynomialLongDivision divides by the divisor's leading coefficient. Its
first attempt, on the pair as written, declines a symbolic one for the
simplifier, since a quotient that loses a value of a parameter is not an
equivalence. The second attempt, with the variables replaced by
temporaries, had no such guard: it predates it, and #1269 left it as it
was. (t x + y)/(1 - y x) divided there in y is
-1/x + (t x + 1/x)/(1 - x y), undefined at x = 0, where the divisor is 1,
and Simplify answered -(t x + y)/(1 - y x) + (t x + y)/(1 - y x) with
0 provided not 1 - x y = 0 and not x = 0. The input is -y + y at x = 0.

The replaced attempt now declines too where the coefficient is symbolic
and the divisor has more than one term in the variable. A divisor of one
term, a x^k, is zero wherever its coefficient is, so dividing by it loses
nothing the input had, and is still done. Guarding that case as well
changed the limit of (tan(a x) - sin(b x))/(a x) at 0 from (a - b)/a to
1 - b/a, because Simplify does not put 1 - b/a over one denominator
(#2137), so it is not guarded.

On 2.6.0 and on this branch:
  -(t*x + y)/(1 - y*x) + (t*x + y)/(1 - y*x)
    0 provided not 1 - x * y = 0 and not x = 0  ->  0 provided not 1 - x * y = 0
Unit suite 17,130 passed, 0 failed. SimpSweep 10,463/10,463 agree;
CanonCheck matches its baseline; BoundCheck 0 disagreements; CasBench
120/121, unchanged.

Part of #2134

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura
@Rafael-SOWNet Rafael-SOWNet added this to the 2.7.0 milestone Oct 27, 2026
@Rafael-SOWNet Rafael-SOWNet moved this from Todo to In review in AngouriMath Oct 27, 2026
@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.41%. Comparing base (689fd01) to head (d91296a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2171      +/-   ##
==========================================
- Coverage   86.42%   86.41%   -0.02%     
==========================================
  Files         295      295              
  Lines       64806    64808       +2     
  Branches    19059    19060       +1     
==========================================
- Hits        56009    56004       -5     
- Misses       4768     4774       +6     
- Partials     4029     4030       +1     
Flag Coverage Δ
cpp 37.77% <ø> (ø)
csharp 86.98% <100.00%> (-0.02%) ⬇️
fsharp 32.97% <ø> (ø)
interactive 40.72% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...AngouriMath/Functions/TreeAnalyzer/LongDivision.cs 90.21% <100.00%> (+0.21%) ⬆️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Rafael-SOWNet
Rafael-SOWNet merged commit 2d7d31e into master Oct 27, 2026
34 of 35 checks passed
@Rafael-SOWNet Rafael-SOWNet moved this from In review to Done in AngouriMath Oct 27, 2026
Rafael-SOWNet added a commit that referenced this pull request Oct 27, 2026
…2172)

With no threshold, codecov/project fails on any fall at all from the base
commit's figure. Now that reports reach Codecov (#2162, #2165), that turns
ordinary changes red: #2171 was at -0.02%, five lines that a new guard
returns before. A threshold of 1% lets that movement through and still
fails what the status is for, such as a test leg whose report goes empty,
which moved the figure from 86.41% to 0.38%. Codecov's validator accepts
the file and reads the threshold back as 1.0.


Claude-Session: https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Rafael-SOWNet
Rafael-SOWNet deleted the a-division-in-placeholders-keeps-the-symbols-values branch October 28, 2026 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant