From a0251e8d65800dd4f3f996542876a56f39286724 Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Sun, 6 Sep 2026 19:44:03 +0000 Subject: [PATCH] Every registered rule set runs as data The three CanonicalOrder sets were the last to run their switch. MatchedRules.Sort(level) had existed as their data form, held to Patterns.SortRules(level) by MatchedRulesAgreeWithTheSwitchTest at a hundred firings per level, and been run by nothing. They are repointed at it, through the constructor that takes a MatchedRuleSet and so runs and describes the same object. 30 of 30 registered sets now execute the matcher and 0 take their Rules from RuleRegistryGenerator; both numbers were 27 and 29 when RuleMetadataTest's remark was written, and it says so now. Correctness at the entry point was measured rather than inferred from the agreement test: 8310 generated inputs simplified on a build of master and on this one, diffed, zero differences. And it is not free, which is the thing the last exchange's write-up says to state. Allocation, master against this commit, one benchmark on one machine: SimplifyHard 3,655,695,192 -> 3,662,375,184 +0.18% SolveHard 1,450,058,856 -> 1,452,704,384 +0.18% SolveMediumHard 165,055,344 -> 165,611,184 +0.34% SimplifyEasy 128,099 -> 128,460 +0.28% Real -- those rows reproduce to a part in a hundred thousand -- and inside PerformanceGate's 3%. The compile rows moved -1.5%, which is inside their own 2% floor and means nothing. The Sort set runs on every node of every pass, so a matcher dispatch where a switch used to be is the likeliest source; it is the price of the set being addressable, named and tiered like the other twenty-seven, and since 2.4.0 this family has now paid roughly half a per cent across three such steps. The repoint tripped the gate it should have. RuleMetadataTest requires that a set is repointed only once every rule of it carries an identity, so the registry gains descriptions rather than trading them -- and the nine Sort rules had none, rendering as `Xorf x => (built by code)`. They carry one each now. The fix is at the source, not the gate. Five census pins moved because the exchange succeeded, each to the number the failure reported: 27 sets describing what they run is 30; 315 addressable rules is 321; 294 rules carrying an identity is 321, in two tests and in WritingARule.md. One pin was wrong while passing: "how many registered sets run the matcher" matched MatchedRules names, and CanonicalOrder runs under the name Sort, so it said 27 while the truth was 30. It has the same special case CommonDenominator already had. StepAsASentenceTest asserted that some set was still generator-described and that its names did not pass for prose. That population is empty now, and the assertion flips to say so rather than being deleted: a set falling back to the generator is a regression this should catch. The audit that prompted this called six MatchedRules sets orphans. Measured: three of those are the CommonDenominator family, wired all along; the other three -- PowerOfPower, PythagoreanIdentity, SharedFactor -- are single-rule fixtures that ReversibleRuleTest, GatheredMatchingTest and the e-match tests use as subjects, and deleting them deletes the evidence. Two duplicate registry rules under near-identical names. SharedFactor's `a-shared-factor-comes-out-of-a-sum` appears nowhere else, which is worth its own look and is not taken here. Part of #825 and #746. Full suite 9571 passed, 0 failed. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura --- .../Transformations/Matching/MatchedRules.cs | 27 ++++++++++++------- .../Core/Transformations/RewriteRules.cs | 9 +++---- .../Docs/Contributing/WritingARule.md | 12 +++++---- .../Transformations/AddressableRulesTest.cs | 2 +- .../Transformations/RuleAuthoringGuideTest.cs | 13 ++++++--- .../Core/Transformations/RuleMetadataTest.cs | 18 ++++++++----- .../Transformations/StepAsASentenceTest.cs | 21 ++++++++------- 7 files changed, 61 insertions(+), 41 deletions(-) diff --git a/Sources/AngouriMath/Core/Transformations/Matching/MatchedRules.cs b/Sources/AngouriMath/Core/Transformations/Matching/MatchedRules.cs index ecfcbfe74..2395e8747 100644 --- a/Sources/AngouriMath/Core/Transformations/Matching/MatchedRules.cs +++ b/Sources/AngouriMath/Core/Transformations/Matching/MatchedRules.cs @@ -2120,14 +2120,16 @@ is var (divided, remainder) MatchPattern.Any("x"), (node, _) => Functions.Patterns.SortAndGroup( node, Sumf.LinearChildren(node), level, (a, b) => a + b), - Soundness.SoundUnderAssumptions), + Soundness.SoundUnderAssumptions, + description: "the terms of a sum are put in canonical order and like terms are grouped"), new MatchedRule( "a-difference-chain-is-sorted-and-grouped", MatchPattern.Any("x"), (node, _) => Functions.Patterns.SortAndGroup( node, Sumf.LinearChildren(node), level, (a, b) => a + b), - Soundness.SoundUnderAssumptions), + Soundness.SoundUnderAssumptions, + description: "a difference is read as a sum, whose terms are put in canonical order and grouped"), new MatchedRule( "a-product-chain-is-sorted-and-grouped", @@ -2136,49 +2138,56 @@ is var (divided, remainder) node, Mulf.LinearChildren(node), level, (a, b) => a * b), // Regrouping reads a quotient as a product with a negative power, which is the // same value wherever the divisor is not zero. - Soundness.SoundUnderAssumptions), + Soundness.SoundUnderAssumptions, + description: "the factors of a product are put in canonical order and like factors are grouped"), new MatchedRule( "a-quotient-chain-is-sorted-and-grouped", MatchPattern.Any("x"), (node, _) => Functions.Patterns.SortAndGroup( node, Mulf.LinearChildren(node), level, (a, b) => a * b), - Soundness.SoundUnderAssumptions), + Soundness.SoundUnderAssumptions, + description: "a quotient is read as a product, whose factors are put in canonical order and grouped"), new MatchedRule( "a-conjunction-chain-is-sorted-and-grouped", MatchPattern.Any("x"), (node, _) => Functions.Patterns.SortAndGroup( node, Andf.LinearChildren(node), level, (a, b) => a & b), - Soundness.Sound), + Soundness.Sound, + description: "the operands of a conjunction are put in canonical order and repeats are grouped"), new MatchedRule( "a-disjunction-chain-is-sorted-and-grouped", MatchPattern.Any("x"), (node, _) => Functions.Patterns.SortAndGroup( node, Orf.LinearChildren(node), level, (a, b) => a | b), - Soundness.Sound), + Soundness.Sound, + description: "the operands of a disjunction are put in canonical order and repeats are grouped"), new MatchedRule( "a-union-chain-is-sorted-and-grouped", MatchPattern.Any("x"), (node, _) => Functions.Patterns.SortAndGroup( node, Set.Unionf.LinearChildren(node), level, (a, b) => a.Unite(b)), - Soundness.Sound), + Soundness.Sound, + description: "the operands of a union are put in canonical order and repeats are grouped"), new MatchedRule( "an-intersection-chain-is-sorted-and-grouped", MatchPattern.Any("x"), (node, _) => Functions.Patterns.SortAndGroup( node, Set.Intersectionf.LinearChildren(node), level, (a, b) => a.Intersect(b)), - Soundness.Sound), + Soundness.Sound, + description: "the operands of an intersection are put in canonical order and repeats are grouped"), new MatchedRule( "an-exclusive-disjunction-chain-is-sorted-and-grouped", MatchPattern.Any("x"), (node, _) => Functions.Patterns.SortAndGroup( node, Xorf.LinearChildren(node), level, (a, b) => a ^ b), - Soundness.Sound)); + Soundness.Sound, + description: "the operands of an exclusive disjunction are put in canonical order and repeats are grouped")); /// /// , as data — again one diff --git a/Sources/AngouriMath/Core/Transformations/RewriteRules.cs b/Sources/AngouriMath/Core/Transformations/RewriteRules.cs index 23ee1cb1a..211c2136a 100644 --- a/Sources/AngouriMath/Core/Transformations/RewriteRules.cs +++ b/Sources/AngouriMath/Core/Transformations/RewriteRules.cs @@ -50,8 +50,7 @@ public static class RewriteRules // Regrouping reads a quotient as a product with a negative power, which is the // same value wherever the divisor is not zero. Soundness.SoundUnderAssumptions, - Patterns.SortRules(TreeAnalyzer.SortLevel.HIGH_LEVEL), - Patterns.SortRulesArms(TreeAnalyzer.SortLevel.HIGH_LEVEL), + Matching.MatchedRules.Sort(TreeAnalyzer.SortLevel.HIGH_LEVEL), isNormalization: true); /// @@ -63,8 +62,7 @@ public static class RewriteRules "Sorts and groups commutative operands, distinguishing terms by their constants too.", TransformationRelation.Equivalence, Soundness.SoundUnderAssumptions, - Patterns.SortRules(TreeAnalyzer.SortLevel.MIDDLE_LEVEL), - Patterns.SortRulesArms(TreeAnalyzer.SortLevel.MIDDLE_LEVEL), + Matching.MatchedRules.Sort(TreeAnalyzer.SortLevel.MIDDLE_LEVEL), isNormalization: true); /// @@ -76,8 +74,7 @@ public static class RewriteRules "Sorts and groups commutative operands by the whole subtree.", TransformationRelation.Equivalence, Soundness.SoundUnderAssumptions, - Patterns.SortRules(TreeAnalyzer.SortLevel.LOW_LEVEL), - Patterns.SortRulesArms(TreeAnalyzer.SortLevel.LOW_LEVEL), + Matching.MatchedRules.Sort(TreeAnalyzer.SortLevel.LOW_LEVEL), isNormalization: true); /// diff --git a/Sources/AngouriMath/Docs/Contributing/WritingARule.md b/Sources/AngouriMath/Docs/Contributing/WritingARule.md index e6bc37c8f..1ded6800b 100644 --- a/Sources/AngouriMath/Docs/Contributing/WritingARule.md +++ b/Sources/AngouriMath/Docs/Contributing/WritingARule.md @@ -20,10 +20,12 @@ effect". This is how to supply those seven things. `Core/Transformations/Matching/MatchedRules.cs`, as a value in a `MatchedRuleSet`. **33** sets and **324** rules live there today. -The `switch` statements in `Functions/Simplification/Patterns` are the older form. Twenty-seven of -the thirty registered sets no longer run theirs, and **none of those twenty-seven describes it any -more either**; the three that remain still run theirs, so describing it is honest -([#825](https://github.com/asc-community/AngouriMath/issues/825)). +The `switch` statements in `Functions/Simplification/Patterns` are the older form. **All thirty +registered sets now run as data and describe what they run**; none executes its `switch` any more. +The last three to move were the `CanonicalOrder` family, repointed at `MatchedRules.Sort(level)` — +which had existed as data, held to the `switch` by `MatchedRulesAgreeWithTheSwitchTest` at a hundred +firings per level, and been run by nothing. The `switch` forms stay because that agreement test is +what keeps the two spellings honest ([#825](https://github.com/asc-community/AngouriMath/issues/825)). **Do not add a rule to a `switch`.** A `switch` arm cannot carry a name, a tier, an identity or a direction, and everything below is about those. @@ -89,7 +91,7 @@ debugged for an afternoon. | `Left.ToString()` | `Divf(var a, Divf(var b, var c))` — how the matcher spells it | Write the identity with `=`, not `->`: it is an equality, and the arrow belongs to the direction the -rule happens to be applied in. **294** rules carry one today; a new rule should. +rule happens to be applied in. **321** rules carry one today; a new rule should. ## The pattern language diff --git a/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs b/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs index 193c29c82..444e780ed 100644 --- a/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs +++ b/Sources/Tests/UnitTests/Core/Transformations/AddressableRulesTest.cs @@ -299,7 +299,7 @@ public void TheRegistryIsAddressableAsFarAsItSaysItIs() // are thirty-three; NumericNeat's sixteen are eleven, six of them being three rules // written once per side a negative factor can sit on; and the two factorial sets are // eight arms each written as three. Every other repointed set is one arm for one rule. - Assert.Equal(315, withRules.Sum(set => set.Rules.Count)); + Assert.Equal(321, withRules.Sum(set => set.Rules.Count)); } /// diff --git a/Sources/Tests/UnitTests/Core/Transformations/RuleAuthoringGuideTest.cs b/Sources/Tests/UnitTests/Core/Transformations/RuleAuthoringGuideTest.cs index f8d34ea28..ba6874d1e 100644 --- a/Sources/Tests/UnitTests/Core/Transformations/RuleAuthoringGuideTest.cs +++ b/Sources/Tests/UnitTests/Core/Transformations/RuleAuthoringGuideTest.cs @@ -45,11 +45,16 @@ public void WhereARuleGoes() Stated(33, MatchedRules.All.Count, "the number of rule sets written as data"); Stated(324, MatchedRules.All.Sum(set => set.Rules.Count), "the number of rules written as data"); Stated(30, RewriteRules.All.Count, "the number of registered sets"); - Stated(27, RewriteRules.All.Count(set => + // Two families register under one name and run a data set under another -- + // CommonDenominator over MatchedRules.CommonDenominator(level), CanonicalOrder over + // MatchedRules.Sort(level) -- so a name match alone undercounts them. This said 27 + // while the truth was 30 for exactly as long as the second family was unlisted here. + Stated(30, RewriteRules.All.Count(set => MatchedRules.All.Any(data => data.Name == set.Name) - || set.Name.StartsWith("CommonDenominator", StringComparison.Ordinal)), + || set.Name.StartsWith("CommonDenominator", StringComparison.Ordinal) + || set.Name.StartsWith("CanonicalOrder", StringComparison.Ordinal)), "how many registered sets run the matcher"); - Stated(27, RewriteRules.All.Count(set => + Stated(30, RewriteRules.All.Count(set => set.Rules.Count > 0 && set.Rules.All(rule => rule.Soundness is not null)), "how many registered sets describe what they run"); } @@ -68,7 +73,7 @@ public void TheNameIsASentence() [Fact] public void TheIdentityIsNotTheName() - => Stated(294, + => Stated(321, MatchedRules.All.SelectMany(set => set.Rules).Count(rule => rule.Description is not null), "how many rules carry an identity"); diff --git a/Sources/Tests/UnitTests/Core/Transformations/RuleMetadataTest.cs b/Sources/Tests/UnitTests/Core/Transformations/RuleMetadataTest.cs index 195864bc6..b18b1d4f8 100644 --- a/Sources/Tests/UnitTests/Core/Transformations/RuleMetadataTest.cs +++ b/Sources/Tests/UnitTests/Core/Transformations/RuleMetadataTest.cs @@ -22,11 +22,14 @@ namespace AngouriMath.Tests.Core.Transformations /// /// /// - /// The registry runs one thing and describes another. 27 of the 30 sets execute - /// , and 29 of them take their - /// from RuleRegistryGenerator reading the - /// switch those sets no longer run. That is - /// #825's open half. + /// The registry used to run one thing and describe another. When this was written, + /// 27 of the 30 sets executed while 29 took their + /// from RuleRegistryGenerator reading a + /// switch they no longer ran — #825's + /// open half. Both numbers are now 30 and 0: every registration passes its + /// to the constructor that takes one, which runs it and + /// describes it from the same object. The last three to move were the CanonicalOrder + /// family, and the counts below moved with them. /// /// /// This file is about the three things that had to be true of @@ -112,6 +115,9 @@ public void ARuleWrittenAsDataCarriesItsTierIntoTheRegistry() var fromData = new[] { nameof(RewriteRules.Boolean), + nameof(RewriteRules.CanonicalOrder), + nameof(RewriteRules.CanonicalOrderCountingConstants), + nameof(RewriteRules.CanonicalOrderExact), nameof(RewriteRules.CollapseMultipleFractions), nameof(RewriteRules.Common), nameof(RewriteRules.CollapseTrigonometricFunctions), @@ -168,7 +174,7 @@ public void EveryRuleOfARepointedSetCarriesItsIdentity() var repointed = RewriteRules.All .Where(set => set.Rules.Count > 0 && set.Rules.All(rule => rule.Soundness is not null)) .ToList(); - Assert.Equal(294, repointed.Sum(set => set.Rules.Count)); + Assert.Equal(321, repointed.Sum(set => set.Rules.Count)); Assert.All(repointed, set => Assert.All(set.Rules, rule => Assert.NotNull(rule.Description))); } diff --git a/Sources/Tests/UnitTests/Core/Transformations/StepAsASentenceTest.cs b/Sources/Tests/UnitTests/Core/Transformations/StepAsASentenceTest.cs index 53fa4905a..bcaa543c0 100644 --- a/Sources/Tests/UnitTests/Core/Transformations/StepAsASentenceTest.cs +++ b/Sources/Tests/UnitTests/Core/Transformations/StepAsASentenceTest.cs @@ -63,19 +63,20 @@ public void EveryRuleWrittenAsDataIsNamedInProse() Assert.All(names, name => Assert.True(Explanation.IsProse(name), $"'{name}' is a rule name that does not read as a phrase in English")); - // And a generated one is not mistaken for prose. Which sets those are moves as the - // registry is repointed set by set, so they are found rather than named: a set whose - // rules carry no tier of their own is one still described by `RuleRegistryGenerator`, - // and every name it gives is a rendered pattern. Naming one instead cost a failure the - // day Trigonometric was repointed, which is the argument for asking. + // And no set is described by `RuleRegistryGenerator` any more. A set whose rules + // carry no tier of their own was one still described by the generator, and every name + // it gave was a rendered pattern rather than prose; this used to assert that such + // sets existed and that their names did not pass for English. The last three -- + // the CanonicalOrder family -- were repointed at `MatchedRules.Sort`, so the + // population is empty, and that is asserted rather than the assertion being deleted: + // a set falling back to the generator would be a regression this should catch. var generated = RewriteRules.All .Where(set => set.Rules.Count > 0 && set.Rules.All(rule => rule.Soundness is null)) - .SelectMany(set => set.Rules) - .Select(rule => rule.Name) + .Select(set => set.Name) .ToList(); - Assert.NotEmpty(generated); - Assert.All(generated, name => Assert.False(Explanation.IsProse(name), - $"'{name}' is a rendered pattern and was taken for prose")); + Assert.True(generated.Count == 0, + "these sets are described by the generator again, and their rule names are " + + "rendered patterns rather than prose: " + string.Join(", ", generated)); } ///