Repository navigation
Four rewrite rules hold only where they are written to - #2434
Open
Rafael-SOWNet wants to merge 4 commits into
Open
Rafael-SOWNet wants to merge 4 commits into
Rafael-SOWNet wants to merge 4 commits into
Conversation
…t holds a^n / b^(c*n) -> (a / b^c)^n and its mirror had no guard. The gathering is a^n / b^n -> (a/b)^n with b^c for a base, so it holds on that rule's condition from #802: n whole, or both bases positive reals. Unguarded, x^n / y^(2n) became (x / y^2)^n, which is 1 at x = 1, y = -1, n = 1/2, where the quotient is -1. Both the data rule and its switch arm now ask the same guard. Found by the v3 definedness specification (work/v3-spec); no issue filed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… are ordered (a > b and b > c) implies a > c was simplified to True, carrying the operands' domains but not that they are ordered. At a = i, b = 1, c = 0 the implication is NaN. The ordered condition cannot be attached as x > x attaches it (#876): at a = i, b = 0, c = 1 the conjunction is false and the implication is True, which True provided a in RR is not. So both rules, and their switch arms, fire only where every operand is known ordered: the real reading, or a known-real operand. Two BooleanSimplify rows pinned True for symbols over the complex plane; they now assert it over the reals, where it holds. Over the complex plane the input is left as written. Found by the v3 definedness specification (work/v3-spec); no issue filed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…umerator arcsin(n / d) -> arccsc(d / n) and its three siblings read n / d as 1 / (d / n), which needs n nonzero. At n = 0 arcsin(0 / x), which is 0 wherever x is not, became arccsc(x / 0), which has no value: the narrowing soundcheck reports. The data rules and their switch arms now refuse a zero numerator. Simplify folds 0 / x before the rules are asked, so no Simplify answer changes; the rewrite is reachable through the public rule registry, Transformation.Rewriting(RewriteRules.Trigonometric), and the test asks it there. Found by the v3 definedness specification (work/v3-spec); no issue filed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…perand sgn(a) * (b * a) / abs(a) -> b provided not a = 0 holds for a real a. The library's sgn(a) is a / abs(a), so off the real line the left side is b * a^2 / abs(a)^2, which is -1 at a = i, b = 1. Realness is a reading, not a condition on where a side is defined, so the rule and its switch arm decide it as the comparison rules do (the real reading, or an operand known real) and do not fire otherwise. Simplify sorts the product before Common is asked and the nested product never reaches the rule, so no Simplify answer changes; the rewrite is reachable through Transformation.Rewriting(RewriteRules.Common), and the test asks it there. Found by the v3 definedness specification (work/v3-spec); no issue filed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2434 +/- ##
==========================================
+ Coverage 86.81% 86.86% +0.04%
==========================================
Files 312 312
Lines 69551 69766 +215
Branches 20316 20387 +71
==========================================
+ Hits 60383 60604 +221
+ Misses 4932 4916 -16
- Partials 4236 4246 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This branch has not been deployed
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 #2428.
Four rewrite rules were wrong as written, each the same on 2.6.0. Each fix is its own commit, and each keeps the hand-written
switchcopy of the rule inPatterns.*.csin step with the data rule, asMatchedRulesAgreeWithTheSwitchTestrequires.a^n / b^(c n) → (a / b^c)^n, and its mirror. Wrong throughSimplify.x^n/y^(2*n)became(x / y^2)^n: at x = 1, y = −1, n = 1/2 the input is −1 and the output 1.x^(2n)/y^n→(x^2/y)^nfails the same way at x = −1, y = 1.a^n/b^n → (a/b)^ncarries (a^n / b^n is gathered into (a/b)^n unconditionally, which is wrong for bases of opposite sign #802): n whole, or both bases positive reals. Gathering numeric bases still works:2^n/3^(2n)→(2/9)^n.(a > b and b > c) implies a > c→True. Wrong throughSimplify: at a = i, b = 1, c = 0 the input is NaN.a in RR, asp or not pdepends on which side thenotis written, and reduces to True where the proposition is undefined #876 does forx > x, would narrow instead: at a = i, b = 0, c = 1 the input isTrue, becauseNaN and FalseisFalse.BooleanSimplify.TestInequalityrows that pinnedx > y > z → x > zfor symbols on the complex plane move to a theory over the reals, where they still hold.arcsin(n/d) → arccsc(d/n)at n = 0. Reachable throughTransformation.Rewriting(RewriteRules.Trigonometric):arcsin(0/x)→arccsc(x/0)turns 0 into NaN, andarccos(0/x)→arcsec(x/0)likewise.Simplifyfolds0/xfirst and is not affected.sgn(a) · a / |a|cancels. Reachable throughTransformation.Rewriting(RewriteRules.Common):sgn(x)*(y*x)/abs(x)→yfor non-real x, where at x = i the input is −y.sgn(a)·a/|a|isa^2/|a|^2, which is 1 only for real a.Simplifyis not affected, because the canonical order flattens the product first.soundcheckon master: 12 widen, 1 narrows, 80 none found. On this branch: 12 widen, 0 narrows, 81 none found. The arcsine rule moved from narrows to none found; the 12 widenings are other rules, deferred to #1019 §14. Each changed answer has a BREAKING-CHANGES.md row measured on the 2.6.0 package, each under its own heading.Checked on this branch:
🤖 Generated with Claude Code