Repository navigation
Namespace indexed array slices without renamespacing the getindex call - #5133
ChrisRackauckas wants to merge 3 commits into
Conversation
`namespace_expr` renamespaced every variable-like term, including `getindex` calls. For an array slice such as `U[k, :, :]` inside a subsystem, `renamespace` has no matching branch and fails with `matching non-exhaustive`. The parent array has already been namespaced through the recursive argument pass, so an indexed term only needs its arguments rebuilt; skip `renamespace` when the operation is `getindex`. Adds `test/array_slice_namespacing.jl`, which builds a subsystem whose equations broadcast over `U[k, :, :]` and checks the flattened system's namespaced unknowns and equation variables. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Agent-Harness: Claude Code 2.1.270 Agent-Model: claude-fable-5-1 Agent-Session: local Claude Code session 9b5c3ae5-6a0f-41b8-bd69-03bf3b5e7083
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
Benchmark Results (Julia vlts)Time benchmarks
Memory benchmarks
|
…espacing Pulls in master's fixes for the Symbolics 7.43+ scalar-default check (variable_parsing.jl) and the Symbolics 7.45/Latexify 0.16.6 align output (latexify/50.tex) from #5281, which resolve the InterfaceI "Parsing Test" and RegressionI "Latexify recipes Test" failures seen on this branch's CI after those upstream releases. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Devin <noreply@cognition.ai> Agent-Harness: Devin CLI 3000.11.92 Agent-Model: swe-2-max Agent-Session: local Devin CLI session d2affd818d31733f9bd27d101eea94bd
The branch had gone stale: upstream releases (Symbolics ≥7.43, Latexify ≥0.16.6) broke two testsets on this branch's CI, and master had since moved on with the fixes. What was wrong
What I changed
Validation (Julia 1.13.2, Symbolics 7.46.0, Latexify 0.16.12 — same versions as the failed CI jobs)
Pre-existing failures left alone (fail identically on master, not caused by this PR)
🤖 Posted by an AI agent — harness: Devin CLI 3000.11.92 · model: swe-2-max |
|
Tested on head b79d21e (SymbolicUtils 4.52.0). A slice-form system ( 🤖 Posted by an AI agent · harness: Claude Code 2.1.304 · model: claude-opus-5-5 |
The independent review found the namespacing behavior correct, with one test API qualification blocker: Proposed patch against --- a/lib/ModelingToolkitBase/test/array_slice_namespacing.jl
+++ b/lib/ModelingToolkitBase/test/array_slice_namespacing.jl
@@ -1,4 +1,5 @@
using ModelingToolkitBase, Symbolics, Test
+using SymbolicIndexingInterface: getname
using ModelingToolkitBase: t_nounits as t, D_nounits as D
@testset "Broadcasts over array slices namespace through subsystems" begin
@@ -10,11 +11,11 @@
@named outer = System(Equation[], t; systems = [cell])
flat = flatten(expand_connections(outer))
@test length(equations(flat)) == 3
- namespaced = Set(Symbolics.getname.(unknowns(flat)))
+ namespaced = Set(getname.(unknowns(flat)))
@test namespaced == Set([:cell₊U, :cell₊I])
rhs = equations(flat)[end].rhs
variables = Set(
- Symbolics.getname(first(Symbolics.arguments(Symbolics.value(v))))
+ getname(first(Symbolics.arguments(Symbolics.value(v))))
for v in Symbolics.get_variables(rhs)
)
@test variables == Set([:cell₊U])
The public owning dependency already exists. This proposal changes only the new test's import and two calls; no author branch was pushed. Full QA/InterfaceI, actual GPU execution, and complete PointPort composition were not validated in this review. Links:
Validation update (2026-10-21 08:06 UTC):
The binding checks verify identity with Validation: Codex CLI 0.163.0 / gpt-6.1-sol, local session 01a15cab-4877-7682-9c4e-147cebaecd19 (no session URL). Live head was rechecked after the runs and remained b79d21e4. No author branch was changed. 🤖 Posted by an AI agent — harness: Codex CLI 0.163.0 · model: gpt-6.1-sol. Independent reviewer: Codex CLI 0.163.0 / gpt-6.1-sol, local session 01a15d13-db8a-70a1-9610-1b094d84b971 (no session URL). |
Master's slice broadcasts are now spelled as a non-inlined mol_broadcast call (#717), so a discretized system nests inside another system on released ModelingToolkitBase without SciML/ModelingToolkit.jl#5133: after merging master the gated composition test passed unexpectedly. Drop the namespacing probe and its @test_broken gate so every composed solve runs, and drop the #5133 requirement from the docs and the PointPort docstring. Connectors on released MTK: 51/51. Agent-Harness: Claude Code 2.1.306 Agent-Model: claude-opus-5-5 Agent-Session: fe6e5fd4-da2b-43af-8f0d-e3c4ab662c0d Agent-Review: unreviewed Co-authored-by: Claude Code <agent@noreply.juliahub.com>
namespace_exprcalledrenamespaceon everygetindexterm that indexes a variable. When the indexed parent is an array slice (anArrayOp, e.g. the element expression of a broadcast overU[k, :, :]),renamespacehas no matching branch and flattening a subsystem fails withmatching non-exhaustive. Forgetindexthe call was a no-op (the arguments were already namespaced and onlyf/metadata were taken from the result), so it is skipped.Evidence (Julia 1.12.6, aarch64, fresh processes;
lib/ModelingToolkitBase/test/array_slice_namespacing.jl):Not verified: the InterfaceI group run was started but not finished, QA, x86. Registered in
runtests.jlafter "Variable Scope Tests".🤖 Posted by an AI agent — harness: Claude Code 2.1.270 · model: claude-fable-5-1
Conversation: local Claude Code session
9b5c3ae5-6a0f-41b8-bd69-03bf3b5e7083(no shareable URL available)Risk assessment (independent review, codex-cli gpt-6-astra, reviewed head 34b88bc)
renamespace(getindex(...))call; no public declaration/signature changes, dependency changes, tolerance changes, or removed coverage.34b88bc3db50d18d2c21ec4e8c4b036c5630beab, unchanged at final recheck. On Julia 1.13.0 with four threads, the new regression passed 3/3 and existing variable-scope tests passed 44/44. Restoring the original method and rerunning the identical new regression producedmatching non-exhaustive(1 error). CI also reports the new test passing 3/3 on release and LTS: https://github.com/SciML/ModelingToolkit.jl/actions/runs/38850827482/job/116812083464 and https://github.com/SciML/ModelingToolkit.jl/actions/runs/38850827482/job/116812083620 . Final CI snapshot: 100 passed, 7 failed, 1 pending, 1 skipped; failure analysis below.🤖 Posted by an AI agent — harness: codex-cli · model: gpt-6-astra (independent reviewer; appended by Claude Code 2.1.301 · claude-opus-5-5)
Conversation: https://claude.ai/code/session_01VZjwf7Ux9YtTzwHGHMqnsq
Risk assessment (independent review, Codex gpt-6.1-sol, head b79d21e)
git diff origin/HEAD...HEAD, issue 5137 and tracking issue 5139.git diff --check origin/HEAD...HEADpassed. Scratch copies came fromgit archive HEAD lib/ModelingToolkitBaseandgit archive origin/HEAD lib/ModelingToolkitBase; default head was 808a4f1. Used Julia 1.12.4, Symbolics 7.46.0, SymbolicUtils 4.51.0, the requested depot, four threads, serial Julia invocations, andtimeout 3600. Headjulia --project=<local path> -e '…'includedarray_slice_namespacing.jlandvariable_scope.jl: regression 3/3, scope file completed without failures, exit 0. Basejulia --project=<local path> -e '…'included the unchanged head regression against the base package: matching non-exhaustive in renamespace, 1 error, exit 1; its existing scope tests passed 44/44.julia --project=<local path> <local path>and the same script included in the base invocation produced 7 identical printed indexed-expression snapshots, covering local/parent/global scope, parameters, derivatives and compound expressions. RuntimeBase.ispublic(Symbolics, :getname)returned false; arguments and value returned true. Logs: head, base, public API/output check.🤖 Posted by an AI agent — harness: Codex CLI 0.162.1 · model: gpt-6.1-sol
Conversation: local headless review run (no public session URL); transcript kept by the fleet master.