insert_rule: find the rule to replace through an index, not by a full scan by tombouctou · Pull Request #2001 · Mathics3/mathics-core

tombouctou · 2026-10-16T07:01:04Z

Fixes #1994.

When an unconditional rule is added, insert_rule looks for an existing rule with the same pattern so that it can be replaced. It does this by calling sameQ against every rule already stored. Building a definition of n rules therefore takes O(n²) time, and loading data files written by Save becomes impractical:

Clear[g];
Do[g[i, 2, 3] = i, {i, 1, 4000}];
AbsoluteTiming[Do[g[i, 2, 3] = i, {i, 4001, 5000}]]   (* 18.7 s on master *)

Change. Each Definition now keeps an index of the unconditional rules in each of its rule lists, keyed by a fingerprint of the rule's pattern.

The fingerprint is the structure of the pattern expression, keeping integers, strings and symbols and putting every other atom in a single bucket. So patterns that are SameQ always get the same fingerprint (Reals of different precision may be SameQ, which is why they share one).
The search: sameQ is called only on the rules in the new rule's bucket, and the rule found is then located in the list by binary search on its own precedence key.
Keeping the index valid: an index belongs to the list object it was built from, and is rebuilt lazily when that list is replaced (ClearAll, DownValues[f] = {...}). Rules removed by Unset are dropped from it when they are met. Definitions pickled before this change get an index on first use.

Why not use the precedence key itself? I tried that first. The key of a left-hand side such as f[1] depends on the Flat/OneIdentity attributes of f, and of any inner heads, at the time the rule is created. So f[1] = a; SetAttributes[f, Flat]; f[1] = b would have kept both rules.

Behaviour is unchanged. I added a temporary check to insert_rule that compared, on every insertion, the rule found through the index with the one the old full scan finds. It reported no difference over the whole pytest suite and the doctests. The new tests in test/core/test_definitions.py cover replacement, replacement after the head or an inner head gets attributes, replacement after Unset, lists set as a whole in an order of their own, and conditional rules, which are never replaced. They pass on master as well. Another test counts sameQ calls when adding a rule to a definition of 500 rules; it fails on master.

Timings:

	master	this PR
1000 assignments after 4000 (above)	18.7 s	0.45 s
first 1000 assignments	2.3 s	0.43 s
a 10 MB file of ~90,000 assignments (written by `Save`)	not finished in 15 min	204 s, all 89,000 values as expected

pytest and the doctests give the same results as on master. The only failures are environmental ones, which also fail on master: XML/ZIP/JPEG FileFormat, and the CLI tests that need mathics on the PATH. mypy is clean on mathics/core/definitions.py.

Unrelated to this PR, but noticed while testing: DownValues[f] = {HoldPattern[f[1]] :> c} stores the left-hand side as HoldPattern[HoldPattern[f[1]]]. So a later f[1] = d adds a second rule instead of replacing it. This happens on master too.

🤖 Generated with Claude Code

… scan When a new unconditional rule is added, insert_rule looked for a rule with the same pattern by calling sameQ on every rule already stored, so building a definition of n rules took O(n^2) time. A package of ~90000 assignments did not load in 15 minutes; 1000 assignments to a symbol that already has 4000 DownValues took 19 s. Each Definition now keeps, for each rule list, an index of its unconditional rules by a fingerprint of their patterns: the structure of the expression with integers, strings and symbols, all other atoms lumped together, so that SameQ patterns always share a fingerprint. sameQ is then called only for the rules in that bucket. The index belongs to the list object it was built for and is rebuilt when the list is replaced (ClearAll, DownValues[f] = ...); rules removed by Unset are dropped from it when met. The precedence key cannot serve as this index: the same left-hand side gets a different key when its head, or an inner head, gets attributes like Flat between two assignments. Behaviour is unchanged; the 1000 assignments now take 0.45 s. Fixes Mathics3#1994. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

mmatera · 2026-10-16T10:09:07Z

+    replacement rules are conditional if the replace attribute is
+    a conditional expression.
+    """
+    return not hasattr(rule, "replace") or rule.replace.has_form(SymbolCondition, 2)


Notice that it would also happen that Condition wraps the pattern too.

Right. This PR only moves the old inline is_conditional out of insert_rule; which rules count as conditional is the same as on master. I checked the three places a Condition can sit, on master and on this branch, with the same results on both:

definitions rules kept

f[x_ /; x > 0] = 1; f[x_ /; x > 0] = 2 (inside an argument) 1, replaced

f[x_] /; x > 0 := 1; f[x_] /; x > 0 := 2 (wrapping the whole left-hand side) 2

f[x_] := 1 /; x > 0; f[x_] := 2 /; x > 0 (on the right-hand side) 2

In the first case the condition is just part of the pattern, so sameQ and the fingerprint both see it, and a repeated definition replaces the old one. In the other two the rule is treated as conditional and never replaced, before and after this PR. As far as I know, WMA replaces a definition whose left-hand side and condition are both the same, so the last two rows may be worth changing. But that would change behavior, so I'd keep it out of this PR, which is meant to be a pure speed-up.

mmatera · 2026-10-16T10:13:12Z

With this PR, and #1994, I would like to wait a little bit. We have a problem with the ordering of rules and with the (lack of a) dispatch rules mechanism. I am working on that problem in a separate branch. However, it is worth getting some fresh ideas on this topic.

tombouctou · 2026-10-16T10:51:46Z

Sure, no hurry. Some background on where this and our other recent PRs come from.

We are trying to run Neil Strickland's Mathematica package for Toda's tables of unstable homotopy groups of spheres (strickland1.org/toda) unmodified in Mathics3:

Toda.m builds a rule base of about 960 relations, adding them one by one.
LambdaAlgebra.m computes in the lambda algebra.
LambdaData.m is a 10 MB file written by Save, with about 90,000 assignments.

Each Mathics3 problem we hit we turn into a small, self-contained fix with its own tests, and we report it here.

This PR is the LambdaData.m case: with the full scan, adding the n-th rule costs O(n), and the file did not finish loading in 15 minutes; with the index it loads in about 200 s.

With all our open fixes applied, Toda.m now loads in full (962 relations, against 510 before) and agrees with an independent Python port:

all 182 stored groups agree;
91 of 92 simplification queries agree, and the last one is the Flat matching bug fixed in Flat patterns: a blank can take several elements before a literal; no element is lost #2011.

If your dispatch branch replaces this mechanism, that's fine with us. We're glad to run these files against it as a benchmark when it's ready: they make a fairly demanding test for rule ordering and lookup. And if it helps, the fingerprint here (head/argument structure, keeping integers, strings and symbols) could be one building block for dispatching.

rocky · 2026-10-16T11:02:46Z

Sure, no hurry. Some background on where this and our other recent PRs come from.

We are trying to run Neil Strickland's Mathematica package for Toda's tables of unstable homotopy groups of spheres (strickland1.org/toda) unmodified in Mathics3:

Toda.m builds a rule base of about 960 relations, adding them one by one.

LambdaAlgebra.m computes in the lambda algebra.

LambdaData.m is a 10 MB file written by Save, with about 90,000 assignments.

Each Mathics3 problem we hit we turn into a small, self-contained fix with its own tests, and we report it here.

This PR is the LambdaData.m case: with the full scan, adding the n-th rule costs O(n), and the file did not finish loading in 15 minutes; with the index it loads in about 200 s.

With all our open fixes applied, Toda.m now loads in full (962 relations, against 510 before) and agrees with an independent Python port:

all 182 stored groups agree;

91 of 92 simplification queries agree, and the last one is the Flat matching bug fixed in Flat patterns: a blank can take several elements before a literal; no element is lost #2011.

If your dispatch branch replaces this mechanism, that's fine with us. We're glad to run these files against it as a benchmark when it's ready: they make a fairly demanding test for rule ordering and lookup. And if it helps, the fingerprint here (head/argument structure, keeping integers, strings and symbols) could be one building block for dispatching.

Thanks for the background. It is awesome that you are doing this; I am grateful you selected Mahics3.

If you or Dr. Strickland want to create a repository in the Mathics3 organization with any of you as an owner, I'd be happy to set that up.

…mPy (#2012) `Sum` and `Product` with integer bounds now add (or multiply) the terms directly. SymPy is used only when the bounds are not integers. Before this change, `IterationFunction.eval_iter` sent every `Sum`/`Product` with step 1 through SymPy. The summand was converted to SymPy, the terms were evaluated by Mathics3, the result was converted to SymPy again, put through `Together`, converted back to Mathics3 and then put through `Cancel`. With integer bounds, all that is needed is to evaluate the terms and add them, as `Table` does. When the terms are large symbolic expressions, the round trip costs far more than the sum itself. ``` In[1]:= Sum[1/(x + i), {i, 1, 2}] master: (3 + 2 x) / ((1 + x) (2 + x)) branch: 1 / (1 + x) + 1 / (2 + x) ``` The new form is, as far as I can tell, also what WMA returns: it does not put an explicit finite sum over a common denominator. Symbolic bounds (`Sum[i, {i, 1, n}]`) and infinite sums go through SymPy as before. Tests: `test_sum_product_bounds` in `test/builtin/test_arithmetic.py`. The full pytest suite and doctests pass. Context: we found this while profiling the lambda-algebra homology computation from the Toda-bracket project we are running on Mathics3 (see #2001). In that workload, SymPy conversion inside `Sum` took about 40% of the profile on small cases. Adding the terms is itself costly, though, so on the same core the end-to-end gain is a few percent. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: R. Bernstein <rocky@users.noreply.github.com>

mmatera · 2026-10-16T22:57:09Z

Concretely, instead of lists, rules should be stored as Dispatch objects to handle both the ordering and the access to the rules. But this PR could work as an intermediate step.

tombouctou · 2026-10-17T05:55:57Z

@mmatera thanks. Agreed: once rules are stored as Dispatch objects, the index here can go away, and the fingerprint (head and argument structure, keeping integers, strings and symbols) could be reused as a dispatch key. As an intermediate step this PR changes no behavior. It only makes adding the n-th rule O(1) instead of O(n), so LambdaData.m (about 90,000 assignments) loads in about 200 s instead of not finishing in 15 minutes. I am happy to rebase it onto your branch, or drop it, whichever suits your work better.

…#2014) A format rule that wraps the expression it formats in `Infix`, `Prefix` or `Postfix` made printing hit `$RecursionLimit`: ``` In[1]:= Format[p[t__]] := Infix[p[t], "~"]; p[a, b] master: $RecursionLimit::reclim: Recursion depth of 200 exceeded. branch: a ~ b ``` Before boxing, `do_format_element` formats every subexpression of the result of a format rule. It therefore formatted `p[a, b]` inside the `Infix` again, which gave the same `Infix`, and so on. `Infix`, `Prefix` and `Postfix` display only the elements of their first argument, never the argument itself. Now only those elements are formatted. Defining a format this way is the usual way to give an expression an operator form, and code written for WMA relies on it: the Toda-bracket package we are running on Mathics3 (see #2001) has `Format[t_o] := Infix[t, "∘"] /; Length[t] > 1`. ``` Format[pLeft[t__]] := Infix[pLeft[t], "~", 300, Left]; pLeft[pLeft[a, b], c] a ~ b ~ c Format[pPre[x_]] := Prefix[pPre[x], "!"]; pPre[a] !a Format[pPost[x_]] := Postfix[pPost[x], "!!"]; pPost[a] a!! ``` Tests: five new cases in `test_layout` in `test/format/test_format.py`. All five fail on master. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: R. Bernstein <rocky@users.noreply.github.com>

rocky · 2026-10-17T10:27:14Z

Because this is really, really, really, really useful, I'd like to merge this now and iterate later. on other improvements later.

rocky · 2026-10-17T10:36:17Z

@mmatera, with the various speedups, I wonder if this is showing any change in the benchmarks you've been running?

mmatera · 2026-10-17T11:50:05Z

@mmatera, with the various speedups, I wonder if this is showing any change in the benchmarks you've been running?

This is the evolution of the benchmark results:

https://mathics3.github.io/mathics-core/cache/index.html

I do not see a clear trend, but probably because the benchmarks do not include tests cases where many rules are attached to the same symbol.
The real test would be to check how much it takes now to load Rubi.

rocky · 2026-10-17T17:18:11Z

@mmatera, with the various speedups, I wonder if this is showing any change in the benchmarks you've been running?

This is the evolution of the benchmark results:

https://mathics3.github.io/mathics-core/cache/index.html

I do not see a clear trend, but probably because the benchmarks do not include tests cases where many rules are attached to the same symbol. The real test would be to check how much it takes now to load Rubi.

Thanks for checking. I guess we have more work to do :-)

I tried Rubi, it feels like that fast part of loading specific files is a little faster. However, the slow part

"Modifying 2817 integration rules to distribute coefficients over sums..." is still slow. So again, more work is needed.

@mmatera

…very rule (#2013) Evaluating `f[x]` tries the down- (or sub-) values of `f` one by one. With n stored values (`f[1] = a`, `f[2] = b`, …), every lookup therefore takes time proportional to n: about 8 ms with 8000 values. Code that memoizes many values becomes quadratic. This PR gives a rule list an index of its *literal* rules, keyed by the structure of the left-hand side. A literal rule is an unconditional rule whose left-hand side is made of symbols, integers and strings, under a head that is not `Orderless`, `Flat` or `OneIdentity`. Only the literal rule that is `SameQ` to the expression, if there is one, is tried. It is tried at its own place among the other rules, which are still tried in order as before, so rule precedence does not change. - **Where a literal rule's place comes from:** the sort keys by which `insert_rule` keeps the list ordered. A list that is not in that order (for example, after `DownValues[f] = {...}`), or in which a literal rule shares its sort key with another rule, is not indexed. - **How the index is kept in step with its list:** the index belongs to the list object it was built for. A list that is replaced (by `Clear` or `DownValues[f] = ...`) gets a new index when one is needed. Inserting a literal rule updates the index; removing a rule, or inserting any other kind of rule, drops it. - **Small lists:** lists of fewer than 8 rules are scanned as before. With 1000 stored values, `f[500]` now takes about 10 µs instead of 1.5 ms. **Checks:** - Tests: `test_stored_values_lookup` in `test/core/test_definitions.py`. - Full pytest and doctests also pass in a debug run that compared every indexed lookup with the full scan; there were 0 mismatches. **Context:** we found this while running the lambda-algebra homology computation from the Toda-bracket project on Mathics3 (see #2001). That code stores tens of thousands of values like `BasisFunction[W][elem] = row`. On one core, the change makes loading the definitions about 10% faster, and the gain grows as values accumulate. @mmatera, you mentioned on #2001 that you are reworking rule ordering and dispatch. If this overlaps with that work, I'm glad to rebase it onto your branch, or to close it in favour of yours. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: R. Bernstein <rocky@users.noreply.github.com>

mmatera reviewed Oct 16, 2026

View reviewed changes

This was referenced Oct 16, 2026

Sum, Product with integer bounds: add the terms, do not go through SymPy #2012

Merged

Find stored values such as f[1] = a through an index, not by trying every rule #2013

Merged

Infix, Prefix, Postfix: do not format their first argument as a whole #2014

Merged

tombouctou mentioned this pull request Oct 17, 2026

Reuse the compiled rule of a rule list kept in a variable (Replace, ReplaceAll, ReplaceRepeated) #2019

Merged

Merge branch 'master' into fix-insert-rule-quadratic

ace5f1e

rocky merged commit 5c89db0 into Mathics3:master Oct 17, 2026
15 checks passed

tombouctou mentioned this pull request Oct 28, 2026

Rule sort refactor #1989

Draft

Navigation Menu

Repository navigation

insert_rule: find the rule to replace through an index, not by a full scan - #2001

insert_rule: find the rule to replace through an index, not by a full scan#2001
rocky merged 2 commits into
Mathics3:masterfrom
tombouctou:fix-insert-rule-quadratic

tombouctou commented Oct 16, 2026

Uh oh!

mmatera Oct 16, 2026

Uh oh!

tombouctou Oct 16, 2026

Uh oh!

mmatera commented Oct 16, 2026

Uh oh!

tombouctou commented Oct 16, 2026

Uh oh!

rocky commented Oct 16, 2026

Uh oh!

mmatera commented Oct 16, 2026

Uh oh!

tombouctou commented Oct 17, 2026

Uh oh!

rocky commented Oct 17, 2026

Uh oh!

Uh oh!

rocky commented Oct 17, 2026

Uh oh!

mmatera commented Oct 17, 2026 •

edited

Loading

Uh oh!

rocky commented Oct 17, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

definitions	rules kept
`f[x_ /; x > 0] = 1; f[x_ /; x > 0] = 2` (inside an argument)	1, replaced
`f[x_] /; x > 0 := 1; f[x_] /; x > 0 := 2` (wrapping the whole left-hand side)	2
`f[x_] := 1 /; x > 0; f[x_] := 2 /; x > 0` (on the right-hand side)	2

Conversation

tombouctou commented Oct 16, 2026

Uh oh!

mmatera Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

tombouctou Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

mmatera commented Oct 16, 2026

Uh oh!

tombouctou commented Oct 16, 2026

Uh oh!

rocky commented Oct 16, 2026

Uh oh!

mmatera commented Oct 16, 2026

Uh oh!

tombouctou commented Oct 17, 2026

Uh oh!

rocky commented Oct 17, 2026

Uh oh!

Uh oh!

rocky commented Oct 17, 2026

Uh oh!

mmatera commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rocky commented Oct 17, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mmatera commented Oct 17, 2026 •

edited

Loading