Repository navigation
Conversation
Uxreplace does not recurse into a replacement, so an Indexed nested in the indices of another one (e.g. in a guard `T[0, I[0, 0, 0], 0] < 1.5`) was left in its n-dimensional form while its pointer was flattened, which does not compile. Resolve the nested substitutions innermost first, both when generating FIndexeds and when binding them to macros
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3065 +/- ##
=======================================
Coverage 84.23% 84.23%
=======================================
Files 258 258
Lines 57070 57091 +21
Branches 4874 4877 +3
=======================================
+ Hits 48072 48091 +19
- Misses 8170 8172 +2
Partials 828 828
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # Uxreplace doesn't recurse into a replacement, so the Indexeds nested in | ||
| # another's indices are substituted here, innermost first. E.g., | ||
| # `T[0, I[0, 0, 0], 0] -> TL0(0, IL0(0, 0, 0), 0)` | ||
| for i in sorted(subs, key=lambda i: len(retrieve_indexed(i.indices, deep=True))): |
There was a problem hiding this comment.
that's the wrong fix, Uxreplace should be fixed instead.
There was a problem hiding this comment.
Agreed the root cause is in uxreplace: _uxreplace (symbolics/manipulation.py:80-83) returns a matched key's replacement verbatim, like xreplace, so with T[0, I[0,0,0], 0] -> TL0(0, I[0,0,0], 0) the inner I[0,0,0] (also a key) is never visited.
The catch is that any fix there turns the mapper from a simultaneous substitution into a transitive one somewhere, and uxreplace has ~86 call sites here and ~60 in PRO, so I'd like your view before picking one. I prototyped two (with the call-site resolution in this PR reverted, both fix the reported case and 3-level nesting, and the same set of suites passes, ~1230 tests):
- A. General: on a match, keep substituting inside the replacement's args (the matched key masked so
{x: x+1}still terminates). Changes simultaneous semantics, e.g.{x: y+1, y: x+1}ona[x,y,z]givesa[x+2, y+2, z]instead ofa[y+1, x+1, z]; ~7% slower on a uxreplace micro-benchmark. - B. Indexed only: register
_uxreplaceforIndexed, and only when key and replacement are bothIndexedsubstitute inside the replacement's indices. Everything else unchanged; the one behaviour change is{a[x,y,z]: b[x,y,z], x: y}ona[x,y,z]now givingb[y,y,z](wasb[x,y,z]). No measurable cost. - C. Opt-in: A's mechanics behind
Uxreplace(subs, nested=True)(Transformer already has anestedkwarg), used only by linearization. No change for anyone else, a bit more plumbing.
I'd lean to B, but you know the callers better. Which would you prefer?
Uxreplacedoes not recurse into a replacement, soT[0, I[0, 0, 0], 0]in a guard keptIn-dimensional while its pointer was flattened, a compile error. Nested substitutions are now resolved innermost first, both when generating FIndexeds and when binding macros.Test:
test_linearize.py::test_nested_indexeds_in_guard(macro and values); reverting either site fails it.Both this PR and #3064 append to the bottom of
test_linearize.py; whichever lands second needs a trivial rebase.🤖 Generated with Claude Code