feat: accept array equations on the implicit-DAE path - #4983
feat: accept array equations on the implicit-DAE path#4983ChrisRackauckas-Claude wants to merge 5 commits into
Conversation
An operator applied to an array variable or a slice, such as `D(u[2:4])`, names the array
rather than its elements. Every caller tests membership of the scalar unknowns, so nothing
matched and the variables were silently misclassified.
The visible consequence is in DAEProblem: `differential_vars` is built as
`map(Base.Fix2(in, collect_differential_variables(sys)), unknowns(sys))`, so a system whose
equations differentiate an array slice reports *every* variable as algebraic.
Before: collect_differential_variables(D(w[2:3]) ~ w[1:2]) == Set([w[2:3]])
membership map over unknowns == [0, 0, 0, 0]
After: Set([w[2], w[3]])
membership map over unknowns == [0, 1, 1, 0]
Scalar operators take the identical path as before.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1dxUEPGysx27SSSpxtiQ9
A finite-difference PDE discretization can emit its interior as a single array equation
over slices rather than one scalar equation per grid point, which keeps the symbolic
system size independent of the grid. `mtkcompile` scalarizes such equations, so the
representation is erased before codegen; `ODEProblem` fundamentally needs that, since it
requires `D(x) = f(x)` and isolating the derivative from a residual is structural
simplification.
The implicit-DAE path does not: a residual `D(u) - f ~ 0` is already the form
`DAEProblem` wants. Four changes let array equations through it, all inert elsewhere:
- `check_array_equations_unknowns` is skipped when `implicit_dae`; every other problem
type still requires scalarized equations.
- `check_eqs_u0` is skipped only when a DAE system actually contains array equations,
where an equation count is not comparable with an unknown count. Scalarized DAEs keep
the check.
- `expand_array_derivatives` rewrites `D(u[2:4])` into the scalar derivatives bound to
the `du` argument, which a derivative of a slice otherwise matches none of.
- `flatten_array_residuals` expands an array-valued residual entry into one output row
per element, indexing the shared expression rather than scalarizing it.
Both codegen helpers return their input unchanged when no equation is array-valued, so
existing implicit-DAE systems generate identical code.
Tests cover the DAE path constructing with the right row count and differential/algebraic
split, the residual evaluating, an end-to-end solve matching the analytic solution of the
heat equation, and `ODEProblem` still rejecting the same system.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1dxUEPGysx27SSSpxtiQ9
`expand_array_derivatives` flattened the scalarized derivative with `vec`, so for an array equation over a 2D slice the substitution produced a length-81 vector against 9x9 surrounding slices and codegen failed with `DimensionMismatch`. Every array equation of two or more dimensions was affected; the 1D tests could not catch it because `vec` is a no-op there. Adds a 2D regression test. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1dxUEPGysx27SSSpxtiQ9
| the corresponding scalar derivatives. Implicit-DAE codegen binds scalar `D(uᵢ)` terms to | ||
| elements of the `du` argument, and a derivative of a slice matches none of them. | ||
| """ | ||
| function expand_array_derivatives(ex) |
There was a problem hiding this comment.
This entire function can be made more efficient. Instead of making half a dozen comments, I'll compile it into this one. _array_derivative_terms! is just SU.search_variables! with is_atomic = x -> isdifferential(x) && SU.is_array_shape(SU.shape(x)) to get all array derivative applications. This removes the entire function and makes it more efficient. The buffer should be a Set{SymbolicT}(). subs should be concretely typed. Instead of the weird collect(scalarize(wrap( this should use SU.stable_eachindex to iterate over the symbolic array and manually build an array_literal. wrap is dynamic dispatch and doesn't infer, as is every function after that point. Substitution should use IRSubstituter. There's no need to wrap the argument to substitute and then unwrap it.
If this function also accepted rhss in its entirety instead of one element at a time, the caching in search_variables! and IRSubstituter is also shared.
There was a problem hiding this comment.
Done — rewritten along all of those lines. _array_derivative_terms! is gone, replaced by search_variables! with an ArrayDerivativeIsAtomic callable (isdifferential(ex) && SU.is_array_shape(SU.shape(ex))); since scalar variables are not atomic under that predicate, nothing else is collected and no post-filter is needed. Buffer is Set{SymbolicT}, subs is Dict{SymbolicT, SymbolicT}, the elements come from SU.stable_eachindex instead of collect(scalarize(wrap(...))), substitution goes through IRSubstituter{false}, and the wrap/unwrap round trip is gone.
It now takes rhss in its entirety, so the search_variables! and IRSubstituter caches are shared across residuals.
One detail worth flagging: the elements have to be reshaped back to the argument shape before building the literal. A derivative of a 2D slice must expand to a 2D array or it stops broadcasting against the surrounding slices — that was a real bug in the previous revision of this PR. I verified stable_eachindex iterates column-major and matches scalarize exactly, including for rank 2.
| into the same underlying expression rather than being scalarized separately, so the array | ||
| computation is built once. | ||
| """ | ||
| function flatten_array_residuals(rhss) |
There was a problem hiding this comment.
This entire function is redundant. generate_rhs should build an ArrayMaker expression where each equation writes to the appropriate contiguous region of the output. That will codegen much better than this approach. For example, a 1D PDE with a body of length n would generate something equivalent to
@makearray out[1:(n+2)] begin
out[1:1] => left_boundary
out[2:(n+1)] => body
out[(n+2):(n+2)] => right_boundary
endExcept generated programmatically with the constructor instead of with a macro.
There was a problem hiding this comment.
Done — flatten_array_residuals is gone, replaced by array_residual_maker, which builds a single ArrayMaker where each residual writes to a contiguous region, exactly as in your sketch but constructed programmatically.
Two constraints I hit that shape the implementation:
ArrayMakerrejects scalar values (AssertionError: sT <: AbstractArray), so a scalar residual is lifted to a length-1 array literal viaSU.Const{VartypeT}([rhs])and given a length-1 region.- The output is a flat residual vector, so a rank > 1 residual cannot be given a multidimensional region. There is no
reshapefor symbolic arrays, andvec(wrap(x))scalarizes — butvecon the unwrappedBasicSymbolicstays symbolic (vec((w(t))[2:3, 4:5]), shape1:4), so that is what is used.
It does codegen much better, as you said. Generated DAEFunction expression size for a 1D heat system, before vs after:
| n | flatten_array_residuals |
ArrayMaker |
|---|---|---|
| 20 | 19,141 | 11,948 |
| 200 | 140,095 | 53,035 |
| 2000 | 1,393,803 | 480,776 |
Still linear in n — the remaining growth is the unknowns/argument handling, not the residual body — but the constant is ~2.9x smaller at n=2000.
| # On the implicit-DAE path an array equation stands for one row per element, so a | ||
| # count of equations is not comparable with a count of unknowns. | ||
| if !(implicit_dae && any(eq -> Symbolics.isarraysymbolic(eq.lhs), eqs)) | ||
| check_eqs_u0(eqs, dvs, u0; check_length, kwargs...) |
There was a problem hiding this comment.
This check can still work if it is refactored slightly to not assume each equation is a scalar.
There was a problem hiding this comment.
Done — the check runs on this path now rather than being skipped. check_eqs_u0 counts residual rows instead of equations: a new count_equation_rows/equation_row_count sums prod(length, shape) for an array equation and 1 otherwise, taking the shape from whichever side carries it (D(u[2:4]) ~ rhs or 0 ~ rhs).
Since array equations previously could not reach check_eqs_u0 at all, this is a strict generalization for every other caller. GROUP=InterfaceI passes: 1595 passed, 0 failed, 5 pre-existing broken.
| # An operator applied to an array variable or slice, such as `D(u[2:4])`, | ||
| # names the array rather than its elements. Callers test membership of the | ||
| # scalar unknowns, so record the elements. | ||
| if symtype(arg) <: AbstractArray |
There was a problem hiding this comment.
SU.is_array_shape(SU.shape(arg)) is a more efficient check.
There was a problem hiding this comment.
Done, using SU.is_array_shape(SU.shape(arg)).
Co-authored-by: Aayush Sabharwal <aayush.sabharwal@gmail.com>
…queries - `expand_array_derivatives` takes every residual at once and uses `search_variables!` with an array-derivative `is_atomic`, a concretely typed `Set`/`Dict`, `stable_eachindex` instead of `collect(scalarize(wrap(...)))`, and an `IRSubstituter`, so the search and substitution caches are shared. - `flatten_array_residuals` is replaced by `array_residual_maker`, which builds a single `ArrayMaker` writing each residual to a contiguous output region rather than indexing out one expression per row. - `check_eqs_u0` counts residual rows rather than equations, so the check runs on the implicit-DAE path instead of being skipped for array equations. - `collect_operator_variables` uses `is_array_shape`/`stable_eachindex`. - Skip array-valued LHSs in `ir_info`, which have no scalar `toterm` name; this makes the `D(u[2:n-1]) ~ rhs` form work, not just the residual form. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
|
Pushed as d6640a0, rebased onto d9c3814 so the accepted suggestion is preserved (that commit applied the Replies are on each thread. Summary:
One thing I found while doing this, which is not from the review. The natural Verification, all local:
|
|
Correction to the codegen numbers above. I measured Counting generated code only (everything before the embedded
The What is still O(n), and why. The remaining growth is entirely The asymmetry comes from I tried the obvious fix — substituting |
|
|
||
| equation_row_count(eq) = 1 | ||
|
|
||
| function equation_row_count(eq::Equation) |
There was a problem hiding this comment.
Just typeassert that shape(eq.lhs)::SU.ShapeVecT and return prod(length, sh; init = 1)
| [`expand_array_derivatives`](@ref) collects `D(u[2:4])` itself rather than descending into | ||
| it. Scalar variables are not atomic here, so nothing else is collected. | ||
| """ | ||
| struct ArrayDerivativeIsAtomic end |
There was a problem hiding this comment.
This shouldn't be a callable singleton struct instead of a normal function?
| function expand_array_derivatives(rhss::Vector{SymbolicT}) | ||
| terms = Set{SymbolicT}() | ||
| for rhs in rhss | ||
| SU.search_variables!(terms, rhs; is_atomic = ArrayDerivativeIsAtomic()) |
There was a problem hiding this comment.
Using IRStructureSearchBuffer here is much faster
| sh = SU.shape(arg) | ||
| # Preserve the shape: a derivative of a 2D slice must expand to a 2D array of | ||
| # scalar derivatives, or it will not broadcast against the surrounding slices. | ||
| sz = ntuple(i -> length(sh[i]), length(sh)) |
There was a problem hiding this comment.
| sz = ntuple(i -> length(sh[i]), length(sh)) | |
| sz = size(arg) |
| # Preserve the shape: a derivative of a 2D slice must expand to a 2D array of | ||
| # scalar derivatives, or it will not broadcast against the surrounding slices. | ||
| sz = ntuple(i -> length(sh[i]), length(sh)) | ||
| els = [op(arg[idx]) for idx in SU.stable_eachindex(arg)] |
There was a problem hiding this comment.
A better method is to build arrargs = Symbolics.SArgsT() and sizehint! it. Then, push sz and each of op(arg[idx]). This allows building Symbolics.STerm(SU.array_literal, arrargs; type = symtype(arg), shape = sh). This avoids the dynamic dispatch reshape.
| subs[term] = SU.Const{VartypeT}(reshape(els, sz)) | ||
| end | ||
|
|
||
| subber = SU.IRSubstituter{false}(IRStructure{VartypeT}(), subs) |
There was a problem hiding this comment.
Why not just reuse get_irstructure(sys)? That way the expression is also prepopulated for codegen to use.
| end | ||
|
|
||
| subber = SU.IRSubstituter{false}(IRStructure{VartypeT}(), subs) | ||
| return map(subber, rhss) |
There was a problem hiding this comment.
Shouldn't need the map iirc. Could also use map! and avoid the allocation.
| values = similar(rhss) | ||
| offset = 0 | ||
| for (i, rhs) in enumerate(rhss) | ||
| sh = SU.shape(rhs) |
There was a problem hiding this comment.
| sh = SU.shape(rhs) | |
| sh = SU.shape(rhs)::SU.ShapeVecT |
For now, since this code assumes it anyway. We'll have to lift this restriction to allow resizing the PDE without redoing codegen.
| if !isempty(assertions(sys)) && !isempty(rhss) | ||
| rhss[end] += unwrap(get_assertions_expr(sys)) | ||
| assertion_expr = unwrap(get_assertions_expr(sys)) | ||
| # An array-valued residual stands for several output rows, and `+` is not defined |
There was a problem hiding this comment.
A future PR should probably make assertions codegen to isoutofdomain?
Note
Draft — please ignore until reviewed by @ChrisRackauckas.
Stacked on #4982 (
collect_operator_variables), which supplies the correctdifferential_varsthis relies on.Why
A finite-difference PDE discretization can emit its interior as one array equation over
slices instead of one scalar equation per grid point, which makes the symbolic system
size independent of grid resolution (SciML/MethodOfLines.jl#428). Measured on the 1D heat
equation, generating code directly from such a system: 1 614 expression nodes at every
grid size, versus 20 646 → 117 446 growing for the scalarized form, with identical
numerics.
mtkcompilescalarizes array equations, so that structure is erased before codegen.ODEProblemfundamentally needs it to: it requiresD(x) = f(x), and isolating thederivative out of a residual is structural simplification.
The implicit-DAE path does not have that requirement — a residual
D(u) - f ~ 0is exactlywhat
DAEProblemconsumes.What changed
Four changes, all inert outside the implicit-DAE path with array equations:
check_array_equations_unknownsimplicit_dae; every other problem type keeps the guardcheck_eqs_u0expand_array_derivativesD(u[2:4])into the scalar derivatives bound to theduargument, which a derivative of a slice otherwise matches none offlatten_array_residualsBoth codegen helpers return their input unchanged when nothing is array-valued, so existing
implicit-DAE systems generate identical code.
Evidence
Before this,
DAEProblemon such a system failed in sequence: the array-equation guard,then
Equations (3), unknowns (21), then a literalDifferentialsurviving into thegenerated code (
MethodError: no method matching (::Differential)(::Vector{Float64})).After, on the heat equation with 3 equations and 21 unknowns:
differential_vars= 19/21 and 79/81 — interior differential, boundaries algebraicDFBDF+BrownFullBasicInit:max|u - exact| = 7.57e-4against
exp(-π²t)sin(πx), the expected second-order spatial error on 21 pointsODEProblemon the same system still throwsIncludes a 2D case:
expand_array_derivativesoriginally flattened the scalarizedderivative with
vec, which produced a length-81 vector against 9×9 surrounding slicesand failed with
DimensionMismatch. Every array equation of two or more dimensions wasaffected, and the 1D tests could not catch it because
vecis a no-op there. Fixed, witha regression test.
Notes for review
DAEProblemstill needsbuild_initializeprob=falsehere: MTK's initialization systembuilds a time-independent system and rejects the
Differentialinside the arrayequation. The solver's own
BrownFullBasicInitcovers that case, but it is only validwhen no initialization equation constrains a derivative or an algebraic variable, so
automating the choice needs care. Not addressed in this PR.
Systemconstruction in residual form. Writtennaturally as
D(u[2:n-1]) ~ rhs,diff2termfails withCan only build StableIndex{Int} from indexed symbolic— it cannot name a derivativeof a slice. Worth knowing for anyone generating such systems.
rows. If it does not, the stencil is recomputed per row, which would be worse than
scalarizing — so the performance claim above is about the symbolic representation,
not yet about generated-code quality.
lib/ModelingToolkitBase/test/array_equation_dae.jl; I ran that file andvariable_utils.jl, not the full suite.🤖 Generated with Claude Code
https://claude.ai/code/session_01S1dxUEPGysx27SSSpxtiQ9