From cf0ac12ac6efa1e022a0695b897d0edcd39f75b9 Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Wed, 26 Aug 2026 17:56:58 +0000 Subject: [PATCH] A bounded pattern is walked by index, not enumerated (#1079) `MatchedRule.TryApply` had two paths: a pattern that `IsDeterministic` is asked for a single match and allocates nothing, and anything else is enumerated through `Match`, which is a chain of iterator state machines, one per pattern node, built at every node of the tree a pass visits. Between the two sits a case that is neither and is common. A `Commutative` node of deterministic children offers the written order and the swapped one and nothing else -- two candidates, known before it is asked. Enumerating those two costs the whole iterator chain, and on a set that runs on every pass it is measurable: `RewriteRules.Power` as data with two commutative rules took `SolveMediumHard` from 165.05 MB to 171.37 MB, +3.8%, past the kernel gate's 3% band, and had to be written out as six node patterns to land (#1076). So a pattern now says how many candidates it can offer at most -- `ChoiceCount`, or `Unbounded` where it cannot say -- and can be asked for the nth of them without an iterator. A node's count is the product over its children, doubled when commutative; `Gathered` is unbounded, because how many ways k parts sit among n operands is a property of the expression rather than of the pattern, and anything containing one is unbounded with it. The index is a mixed radix over the children with the first the most significant digit, because `MatchInOrder` makes it the outermost loop -- so indexing yields the candidates in the order enumeration yields them, which matters: `TryApply` takes the first that satisfies the rule's `when`, so two implementations agreeing on the set of matches and differing on the order are two different rewriters. The count is an upper bound rather than a count, since a child whose name is already bound offers one candidate or none depending on what it is asked to match; an index that does not exist answers false and is skipped, exactly as an enumeration omits it. `BoundedMatchingAgreesWithEnumeration` holds the two implementations together over every rule in `MatchedRules` and the corpus already there, asserting count, order and bindings, and asserting that at least one pattern did match more than one way -- or it would be the deterministic test again under another name. Measured on master, with no new rule set: `SolveMediumHard` 165,054,016 B to 163,400,736 B, -1.00%. The sets already converted carry commutative patterns, so the saving is there to collect before anything else is exchanged. The set list in `DeterministicMatchingTest` was five names, written when there were five and still five while `MatchedRules` grew past twenty -- so the test whose subject is "every rule" was looking at a tenth of them. It reads the class now. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd --- .../Transformations/Matching/MatchPattern.cs | 139 ++++++++++++++++++ .../Transformations/Matching/MatchedRule.cs | 20 +++ .../DeterministicMatchingTest.cs | 84 ++++++++++- 3 files changed, 236 insertions(+), 7 deletions(-) diff --git a/Sources/AngouriMath/Core/Transformations/Matching/MatchPattern.cs b/Sources/AngouriMath/Core/Transformations/Matching/MatchPattern.cs index 13f05f38c..bae2d58d2 100644 --- a/Sources/AngouriMath/Core/Transformations/Matching/MatchPattern.cs +++ b/Sources/AngouriMath/Core/Transformations/Matching/MatchPattern.cs @@ -193,6 +193,66 @@ internal bool TryMatchOnce(Entity expr, Bindings bindings, out Bindings result) private protected abstract bool TryMatchOnceCore( Entity expr, Bindings bindings, out Bindings result); + /// + /// How many candidate matches this pattern can offer at most, or + /// when it cannot say. + /// + /// + /// + /// Between — exactly one — and — + /// however many — sits the case that is neither and is very common: a + /// node of deterministic children, which offers the written + /// order and the swapped one and nothing else. Enumerating two candidates through + /// allocates an iterator state machine per pattern node, and a + /// rewrite pass makes an attempt at every node of the tree, so on a set that runs on + /// every pass that is measurable: 165.05 MB to 171.37 MB of SolveMediumHard, +3.8%, + /// for two commutative rules in RewriteRules.Power. + /// #1079 + /// + /// + /// This is an upper bound, not a count: a child whose name is already bound offers + /// one candidate or none depending on what it is asked to match, which is not known until + /// it is asked. answers for an index + /// that does not exist, so a caller walks every index and skips the misses. + /// + /// + internal virtual int ChoiceCount => 1; + + /// A pattern that cannot bound its candidates, and must be enumerated. + internal const int Unbounded = 0; + + /// + /// The th way this matches, counted the way + /// yields them — so choice i is the ith element of that + /// sequence, once the indices that do not exist are skipped. + /// + /// + /// It must agree with in content and in order. + /// BoundedMatchingAgreesWithEnumeration is the test that holds the two together + /// over generated expressions, for the reason the deterministic path has one: two + /// implementations of one thing is how a matcher acquires a case where they differ. + /// + internal bool TryMatchChoice(Entity expr, Bindings bindings, int choice, out Bindings result) + { + if (RootType is { } required && !required.IsInstanceOfType(expr)) + { + result = bindings; + return false; + } + return TryMatchChoiceCore(expr, bindings, choice, out result); + } + + /// + /// One candidate by index. The default is the deterministic one, which is right for every + /// pattern that offers a single match; overrides it. + /// + private protected virtual bool TryMatchChoiceCore( + Entity expr, Bindings bindings, int choice, out Bindings result) + { + result = bindings; + return choice == 0 && TryMatchOnceCore(expr, bindings, out result); + } + /// The names this pattern binds, so a right-hand side can be checked for a typo. internal abstract IEnumerable BoundNames { get; } @@ -574,6 +634,78 @@ private protected override bool TryMatchOnceCore( return true; } + /// + /// The product over the children, doubled for a commutative node because it offers + /// the written order and the swapped one. as soon as one + /// child is, which is how a anywhere inside makes the + /// whole pattern something to enumerate. + /// + internal override int ChoiceCount + { + get + { + if (choices != -1) return choices; + long total = 1; + foreach (var child in children) + { + var count = child.ChoiceCount; + if (count == Unbounded) return choices = Unbounded; + total *= count; + if (total > MaxChoices) return choices = Unbounded; + } + if (commutative) total *= 2; + return choices = total > MaxChoices ? Unbounded : (int)total; + } + } + + /// + /// Past this a pattern is treated as unbounded. It is not a correctness limit — the + /// indexing is exact at any size — but a bound past which walking every index is no + /// longer obviously cheaper than enumerating, and a guard against a pattern whose + /// product overflows. + /// + private const int MaxChoices = 64; + + private int choices = -1; + + private protected override bool TryMatchChoiceCore( + Entity expr, Bindings bindings, int choice, out Bindings result) + { + result = bindings; + var actual = expr.DirectChildren; + if (actual.Count != children.Length) return false; + + // `MatchCore` yields every solution in the written order first and then, for a + // commutative node, every solution in the swapped one -- so the low half of the + // index space is the written order and the high half is the swapped one. + var perOrder = ChoiceCount; + if (perOrder == Unbounded) return false; + var swapped = false; + if (commutative) + { + perOrder /= 2; + if (choice >= perOrder) { swapped = true; choice -= perOrder; } + } + + if (choice < 0 || choice >= perOrder) return false; + + // Mixed radix over the children, with the first child the most significant digit + // -- because `MatchInOrder` makes it the outermost loop, so it is the one that + // varies slowest in the sequence this has to agree with. The suffix product is + // recomputed rather than stored: there are one to three children, and an array + // here would be the allocation this whole path exists to avoid. + for (var i = 0; i < children.Length; i++) + { + var suffix = 1; + for (var j = i + 1; j < children.Length; j++) suffix *= children[j].ChoiceCount; + var digit = choice / suffix % children[i].ChoiceCount; + var against = swapped ? actual[children.Length - 1 - i] : actual[i]; + if (!children[i].TryMatchChoice(against, result, digit, out result)) + return false; + } + return true; + } + internal override bool IsBuildable => buildable; internal override bool TryBuild(Bindings bindings, out Entity built) @@ -703,6 +835,13 @@ private Entity Leftover(List operands, bool[] used) /// Choosing which operands the parts claim is the whole of what it does. internal override bool IsDeterministic => false; + /// + /// Unbounded, and that is the point of it: how many ways k parts sit among n operands + /// is a property of the expression rather than of the pattern, so this is the one + /// shape that has to be enumerated. + /// + internal override int ChoiceCount => Unbounded; + private protected override bool TryMatchOnceCore( Entity expr, Bindings bindings, out Bindings result) => throw new InvalidOperationException( diff --git a/Sources/AngouriMath/Core/Transformations/Matching/MatchedRule.cs b/Sources/AngouriMath/Core/Transformations/Matching/MatchedRule.cs index 5f300b24f..c0da47704 100644 --- a/Sources/AngouriMath/Core/Transformations/Matching/MatchedRule.cs +++ b/Sources/AngouriMath/Core/Transformations/Matching/MatchedRule.cs @@ -184,6 +184,26 @@ private MatchedRule( return Build(expr, only); } + // Between one match and however many sits the case that is neither and is common: + // a commutative node of deterministic children offers the written order and the + // swapped one and nothing else. Walking those by index costs nothing, where + // enumerating them allocates an iterator state machine per pattern node at every + // node of the tree -- 6.3 MB on `SolveMediumHard` for two commutative rules in + // `RewriteRules.Power`. https://github.com/asc-community/AngouriMath/issues/1079 + // + // The index is an upper bound rather than a count, so a candidate that does not + // exist answers false and is skipped, exactly as an enumeration would omit it. + if (Left.ChoiceCount is var choices and not MatchPattern.Unbounded) + { + for (var choice = 0; choice < choices; choice++) + { + if (!Left.TryMatchChoice(expr, Bindings.Empty, choice, out var bound)) continue; + if (when is not null && !when(bound)) continue; + if (Build(expr, bound) is { } rewritten) return rewritten; + } + return null; + } + // Every way the pattern matches, in order, and the first that also satisfies the // side condition wins. Taking only the first *match* would be wrong: commutativity // means `b*a + c*a` matches `k*p + k*q` several ways and only some of them bind diff --git a/Sources/Tests/UnitTests/Core/Transformations/DeterministicMatchingTest.cs b/Sources/Tests/UnitTests/Core/Transformations/DeterministicMatchingTest.cs index 325c86eec..654958432 100644 --- a/Sources/Tests/UnitTests/Core/Transformations/DeterministicMatchingTest.cs +++ b/Sources/Tests/UnitTests/Core/Transformations/DeterministicMatchingTest.cs @@ -5,8 +5,12 @@ // Website: https://am.angouri.org. // +using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; +using AngouriMath.Core.Transformations; +using AngouriMath.Functions; using AngouriMath; using AngouriMath.Core.Transformations.Matching; using AngouriMath.Extensions; @@ -30,14 +34,30 @@ namespace AngouriMath.Tests.Core.Transformations [Trait("Area", "Core")] public sealed class DeterministicMatchingTest { - private static IEnumerable AllSets => new[] + /// + /// Every set in , read off the class rather than listed. + /// + /// + /// It was a list of five, written when there were five, and it stayed five while the + /// class grew past twenty — so the test whose subject is "every rule" was looking at a + /// tenth of them. A list of names cannot notice that it is out of date; this can. + /// + private static IEnumerable AllSets { - MatchedRules.DivisionPreparing, - MatchedRules.CollapseMultipleFractions, - MatchedRules.PowerOfPower, - MatchedRules.SharedFactor, - MatchedRules.PythagoreanIdentity, - }; + get + { + const BindingFlags Any = BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static; + foreach (var property in typeof(MatchedRules).GetProperties(Any)) + if (property.PropertyType == typeof(MatchedRuleSet)) + yield return (MatchedRuleSet)property.GetValue(null)!; + foreach (var factory in typeof(MatchedRules).GetMethods(Any)) + if (factory.ReturnType == typeof(MatchedRuleSet) + && factory.GetParameters() is { Length: 1 } parameters + && parameters[0].ParameterType == typeof(TreeAnalyzer.SortLevel)) + foreach (var level in Enum.GetValues(typeof(TreeAnalyzer.SortLevel))) + yield return (MatchedRuleSet)factory.Invoke(null, new[] { level })!; + } + } /// /// Shapes chosen to hit the rules and to miss them, including the node types they are @@ -91,6 +111,56 @@ public void DeterministicMatchingAgreesWithEnumeration() Assert.True(checkedPairs > 100, $"only {checkedPairs} pattern/expression pairs checked"); } + /// + /// The same contract one step out: where a pattern can bound its candidates, walking them + /// by index must produce exactly the sequence enumerating them produces — same bindings, + /// same order, same count. + /// + /// + /// Order is asserted and not only membership. TryApply takes the first candidate + /// that also satisfies the rule's when, so two implementations that agree on the + /// set of matches and differ on which comes first are two different rewriters. + /// #1079 + /// + [Fact] + public void BoundedMatchingAgreesWithEnumeration() + { + var checkedPairs = 0; + var sawSeveral = 0; + foreach (var rule in AllSets.SelectMany(set => set.Rules)) + { + var choices = rule.Left.ChoiceCount; + if (choices == MatchPattern.Unbounded) continue; + foreach (var text in Corpus) + { + var expr = text.ToEntity(); + var enumerated = rule.Left.Match(expr, Bindings.Empty).ToList(); + + var byIndex = new List(); + for (var choice = 0; choice < choices; choice++) + if (rule.Left.TryMatchChoice(expr, Bindings.Empty, choice, out var bound)) + byIndex.Add(bound); + + Assert.True(enumerated.Count == byIndex.Count, + $"{rule.Name} on '{text}': enumeration found {enumerated.Count} matches, " + + $"indexing found {byIndex.Count}"); + for (var i = 0; i < enumerated.Count; i++) + foreach (var name in rule.Left.BoundNames.Distinct()) + { + Assert.True(enumerated[i].TryGet(name, out var slow)); + Assert.True(byIndex[i].TryGet(name, out var fast)); + Assert.Equal(slow, fast); + } + if (enumerated.Count > 1) sawSeveral++; + checkedPairs++; + } + } + Assert.True(checkedPairs > 500, $"only {checkedPairs} pattern/expression pairs checked"); + // The whole point is the pattern that matches more than one way. If none of them did, + // this would be the deterministic test again under another name. + Assert.True(sawSeveral > 0, "no bounded pattern matched an expression more than one way"); + } + // That the sets still *rewrite* the same thing end to end is not restated here: // MatchedRulesAgreeWithTheSwitchTest already runs both the data sets and the `switch` // they mirror over generated expressions and requires them to agree, and it now goes