Skip to content

Preserve array flow balances when expanding connections - #5135

Draft
ChrisRackauckas wants to merge 4 commits into
masterfrom
preserve-array-flow-balances
Draft

ChrisRackauckas wants to merge 4 commits into
masterfrom
preserve-array-flow-balances

Conversation

@ChrisRackauckas

@ChrisRackauckas ChrisRackauckas commented Sep 13, 2026 •

Copy link
Copy Markdown
Member

🚧 UNREVIEWED WORK IN PROGRESS — awaiting review by @ChrisRackauckas. Pushed by an AI agent running as @ChrisRackauckas so the work is not lost; Chris has not reviewed it. Please ignore this draft until he does.
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)

expand_connections kept array potential equalities but expanded an array flow balance into one scalar equation per element. When every flow variable in a connection set is an array of the same shape, this generates one array conservation equation instead, so the equation count of a model with array connectors no longer grows with the array size. Fixes #5119.

Evidence (Julia 1.12.6, aarch64, fresh processes; lib/ModelingToolkitBase/test/array_flow_connections.jl):

master:  Array flow connections retain equation blocks | 1248 passed / 2 failed   (allequal(counts), allequal(sizes))
branch:  Array flow connections retain equation blocks | 1250 passed / 1250

Not verified: full test groups and the QA group (earlier notes on this change mention QA failures still to be baselined against master), x86.


🤖 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 4163092)

  • Risk: medium
  • Blast radius: Equal-shape array-valued flow connectors now yield one array conservation equation from exported expand_connections. Scalar flow variables and mixed-shape fallback retain their implementation. Consumers inspecting raw equation blocks see a representation change; the conservation equations remain equivalent after scalarization.
  • Evidence: Reviewed head 416309253df9052513d6f06d9d97c0d0255a3ac1, unchanged at final check. Julia 1.13.0, four threads: new regression 1250/1250 passed; restoring the exact original method from the merge's base parent yielded 1248 passed / 2 failed; independent scalar/array, singleton/multiport, and nested inner/outer parity probe 20/20 passed; modified existing array-stream assertion 1/1 passed. Final CI snapshot: 99 passed, seven failed, one pending, one cancelled, one skipped; failed logs match default-branch failures as detailed below. No PR-caused CI regression identified.
  • Independent review: codex-cli 0.163.0 gpt-6-astra rated it medium, high confidence in the targeted correction: discriminating regression and exact scalarized parity support the implementation; raw public equation representation changes and broad local integration testing remains incomplete.
  • Merge: needs human review — medium risk from the equation-representation change; cancelled/pending jobs and full connector integration are not passing evidence.

🤖 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

🤖 Automated review section from an AI agent running as Chris Rackauckas's account — not written or reviewed by Chris.

Risk assessment (independent review, Codex gpt-6.1-sol, head 4602416)

  • Verdict: MERGE
  • Rating: Codex gpt-6.1-sol rated it medium (M4) at head 4602416
  • Risk: medium (M4: exported expand_connections changes the equation-block representation of working array-flow connections; its documentation does not promise scalar flow equations, making this a representation-design judgment rather than a documented-contract correction).
  • Blast radius: Equal-shape array-flow connection sets, including singleton and nested inner/outer sets, produce array equations. Consumers inspecting raw equation counts or assuming scalar flow equations can notice. Scalar flows retain their implementation; the elementwise conservation equations are preserved. This reaches array-connector preprocessing used before compilation, with no demonstrated change to compiled numerical results or default algorithms. The highest matched criterion is M4; L2 also applies to the regression coverage.
  • Evidence: Read the linked issue, complete git diff origin/HEAD...HEAD, surrounding connection generation and documentation, and risk.md in full. Confirmed HEAD is 46024162458969c779f74b954408a9f359dd008a; git diff --check origin/HEAD...HEAD passed and checkout status remained clean. Local Julia 1.12.4, four Julia threads, Symbolics 7.46.0 / SymbolicUtils 4.51.0: the new test passed 1250/1250 on head; the identical test against archived origin/HEAD 808a4f13e865cf457396ed72ed7d5e7a9ae2a4f8 failed only allequal(counts) and allequal(sizes) (1248 pass, 2 fail). An independent probe comparing exact scalarized symbolic residuals with the original method passed 42/42 checks across scalar/array ports, singleton/two/four-port sets and nested inner/outer connections. CI snapshot: 104 success, 3 failure, 1 cancelled, 1 skipped. G1 pass: the regression checks conservation and thermal constitutive equations against independently specified 300 K, ±3.7 flow and the stated constitutive law; symbolic comparisons additionally establish equivalence to the original elementwise implementation. G2 not applicable: no GPU kernels, AD rules, Reactant or MPI implementation changed; changed CPU connection paths ran locally and the InterfaceI CI jobs passed. G3 not applicable: no shared state, cache, threading or global-default change.
  • Merge: merge — no blocking findings. Medium risk remains subject to Chris's decision under the risk policy; this is not an automatic-merge authorization.

🤖 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.

`expand_connections` expanded an array flow balance into one scalar
equation per element, so a network with array-valued flow connectors
grew its symbolic equation count with the array size (4 -> 103 equation
blocks from 1 to 100 rows in the reproducer of #5119). When every flow
variable in a connection set has the same shape, emit a single broadcast
conservation equation instead. Connection sets with mixed axes keep the
existing element-wise construction.

The existing stream-connector assertion compared against scalarized
equations, so it now scalarizes the array flow equation before comparing.
Adds `test/array_flow_connections.jl`, which checks that the connection
equation count and symbolic size are independent of the array shape and
that the generated equations still express flow conservation.

Fixes #5119.

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
@github-actions

github-actions Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Benchmark Results (Julia v1)

Time benchmarks
master f7fa8d6... master / f7fa8d6...
ODEProblem 4.86 ± 0.18 ms 5.3 ± 0.18 ms 0.917 ± 0.047
init 0.0452 ± 0.016 ms 0.0458 ± 0.016 ms 0.989 ± 0.48
large_parameter_init/ODEProblem 13 ± 0.64 ms 14.4 ± 0.71 ms 0.908 ± 0.063
large_parameter_init/init 0.0583 ± 0.009 ms 0.0593 ± 0.0085 ms 0.983 ± 0.21
mtkcompile 8.39 ± 0.57 ms 9.1 ± 0.5 ms 0.922 ± 0.08
sparse_analytical_jacobian/ODEProblem 15.9 ± 0.86 ms 17.3 ± 0.92 ms 0.921 ± 0.07
sparse_analytical_jacobian/f_iip 0.058 ± 0.002 μs 0.059 ± 0.001 μs 0.983 ± 0.038
sparse_analytical_jacobian/f_oop 0.104 ± 0.0062 ms 0.106 ± 0.0085 ms 0.98 ± 0.098
time_to_load 6.08 ± 0.032 s 6.24 ± 0.046 s 0.975 ± 0.0089
Memory benchmarks
master f7fa8d6... master / f7fa8d6...
ODEProblem 0.0333 M allocs: 1.58 MB 0.0333 M allocs: 1.56 MB 1.01
init 0.477 k allocs: 0.0602 MB 0.477 k allocs: 0.0602 MB 1
large_parameter_init/ODEProblem 0.139 M allocs: 4.61 MB 0.139 M allocs: 4.61 MB 1
large_parameter_init/init 0.759 k allocs: 0.166 MB 0.759 k allocs: 0.166 MB 1
mtkcompile 0.0584 M allocs: 2.71 MB 0.0584 M allocs: 2.7 MB 1.01
sparse_analytical_jacobian/ODEProblem 0.102 M allocs: 4.26 MB 0.102 M allocs: 4.26 MB 1
sparse_analytical_jacobian/f_iip 0 allocs: 0 B 0 allocs: 0 B
sparse_analytical_jacobian/f_oop 0.848 k allocs: 27 kB 0.848 k allocs: 27 kB 1
time_to_load 0.2 k allocs: 11.7 kB 0.2 k allocs: 11.7 kB 1

@github-actions

github-actions Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Benchmark Results (Julia vlts)

Time benchmarks
master f7fa8d6... master / f7fa8d6...
ODEProblem 5.68 ± 0.11 ms 5.75 ± 0.083 ms 0.987 ± 0.024
init 0.0622 ± 0.022 ms 0.0737 ± 0.021 ms 0.844 ± 0.39
large_parameter_init/ODEProblem 15.5 ± 0.41 ms 15.4 ± 0.31 ms 1 ± 0.033
large_parameter_init/init 0.0745 ± 0.018 ms 0.0775 ± 0.018 ms 0.962 ± 0.33
mtkcompile 9.63 ± 0.27 ms 9.7 ± 0.24 ms 0.993 ± 0.037
sparse_analytical_jacobian/ODEProblem 18.7 ± 0.66 ms 18.7 ± 0.41 ms 0.997 ± 0.041
sparse_analytical_jacobian/f_iip 0.077 ± 0.001 μs 0.078 ± 0.001 μs 0.987 ± 0.018
sparse_analytical_jacobian/f_oop 0.375 ± 0.0067 ms 0.375 ± 0.0064 ms 0.999 ± 0.025
time_to_load 7.6 ± 0.069 s 7.57 ± 0.045 s 1 ± 0.011
Memory benchmarks
master f7fa8d6... master / f7fa8d6...
ODEProblem 0.0349 M allocs: 2.01 MB 0.0349 M allocs: 1.99 MB 1.01
init 0.486 k allocs: 0.0779 MB 0.486 k allocs: 0.0779 MB 1
large_parameter_init/ODEProblem 0.145 M allocs: 5.42 MB 0.145 M allocs: 5.42 MB 1
large_parameter_init/init 0.618 k allocs: 0.172 MB 0.618 k allocs: 0.172 MB 1
mtkcompile 0.0624 M allocs: 3.51 MB 0.0624 M allocs: 3.5 MB 1
sparse_analytical_jacobian/ODEProblem 0.106 M allocs: 5.32 MB 0.106 M allocs: 5.32 MB 1
sparse_analytical_jacobian/f_iip 0 allocs: 0 B 0 allocs: 0 B
sparse_analytical_jacobian/f_oop 0.634 k allocs: 19.6 kB 0.634 k allocs: 19.6 kB 1
time_to_load 0.153 k allocs: 14.5 kB 0.153 k allocs: 14.5 kB 1

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

This PR's commit has been carried forward, with its authorship kept, onto current master in #5239. That PR has fresh fail-before/pass-after evidence and an independent review. The review found the change correct but flagged a design decision: array flow balances become broadcast equations. If #5239 is the one to continue, this one can be closed.

🤖 Generated with Claude Code (model: claude-opus-5-5[1m]), https://claude.ai/code/session_01H4fXBChQshAwfpLMCpEvAX

ChrisRackauckas and others added 2 commits October 16, 2026 04:28
Brings in the Symbolics 7.45/Latexify 0.16.6 compat floors and the
variable_parsing/latexify reference updates from #5281, which resolve
the InterfaceI (variable_parsing.jl) and RegressionI (50.tex)
failures seen on the previous CI run.

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 misty-bergamot
@ChrisRackauckas

Copy link
Copy Markdown
Member Author

🤖 Automated comment from an AI agent running as @ChrisRackauckas — not written or reviewed by Chris.

The branch had gone stale against master. What was wrong on the previous head (4163092):

  • sublibrary-ci / lib/ModelingToolkitBase [InterfaceI] (Julia 1 and lts): "Parsing Test" errored with ArgumentError: Variable x must have default of matching size at variable_parsing.jl:66 — Symbolics 7.43 began rejecting scalar variables with vector defaults. Fixed on master by Update MTKBase tests and compat floors for Symbolics 7.43-7.45 #5281 (06b51a13be).
  • sublibrary-ci / lib/ModelingToolkitBase [RegressionI] (Julia 1 and lts): 50.tex latexify reference mismatch from Symbolics 7.43/7.45 output changes — same fix on master.
  • tests / Optimization (julia 1): cancelled at the 2h timeout mid-run. The same test files passed under sublibrary-ci [Optimization] / Julia 1 on this same commit (51m40s).
  • downgrade-mtkbase (Optimization): an Ipopt solve looped in feasibility restoration past iteration 1700 until the 6h limit. Same files passed elsewhere on the same commit. No Optimization test uses array Flow connectors, so the changed code path is unreachable there — this is a dependency-resolution/NLP flake, not something this PR's diff can cause.
  • build (Documentation): Documenter linkcheck failure — also fails on master.
  • SciMLBase.jl/SymbolicIndexingInterface: BoundsError in comprehensive_indexing.jl — fails identically on master (still failing on 9082f85877).
  • SciMLSensitivity.jl/Core8: BoundsError in downstream mtk.jl — also failed on master at the merge base; current master passes it.

What I changed: merged origin/master (9082f85877) into the branch — clean merge, no conflicts, no rebase. That picks up the #5281 test/compat fixes plus the other master fixes; the PR diff vs master is unchanged (4 files, +75/−4).

Local validation on the merged branch (4602416245, Julia 1.13.2 via +release, JULIA_NUM_THREADS=4 JULIA_NUM_PRECOMPILE_TASKS=4, Pkg.test() inside lib/ModelingToolkitBase):

  • GROUP=InterfaceI → InterfaceI | 3053 passed / 5 broken / 3058 in 11m55s (the 5 broken are pre-existing @test_broken); includes the new array_flow_connections.jl and the modified stream_connectors.jl.
  • GROUP=RegressionI → Latexify recipes Test | 1/1 in 15s.
  • GROUP=QA → JET Tests 54/54, Aqua Tests 21/21.
  • GROUP=Optimization → OptimizationSystem 179 pass / 1 broken / 180, MultiObjective 35/35, InfiniteOpt 5/5, Dynamic Optimization Collocation Solvers 146/146; whole group in ~10 min — no hang.
  • array_flow_connections.jl standalone → 1250/1250.
  • julia +release --project=@runic -m Runic --check on the changed Julia files → clean.
  • typos over the diff → clean.

Pre-existing failures left alone: build (docs linkcheck) and SciMLBase.jl/SymbolicIndexingInterface both fail identically on current master. SciMLSensitivity.jl/Core8 failed on master at this branch's merge base and passes on current master; it is a downstream integration job, so a fresh CI run on the merge will tell whether it is resolved here too.


🤖 Posted by an AI agent — harness: Devin CLI 3000.11.92 · model: swe-2-max
Conversation: local Devin CLI session misty-bergamot

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Codex <noreply@openai.com>
Agent-Harness: Codex CLI
Agent-Model: gpt-6-luna
Agent-Session: local fixer job SciML__ModelingToolkit.jl__5135 (no session URL)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot-generated Filed by an AI agent needs-chris-review Bot-filed finding awaiting review by ChrisRackauckas

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve array flow balances when expanding ordinary connector connections

2 participants