Skip to content

Commit 01a7e79

Browse files
ChrisRackauckas-ClaudeChrisRackauckasclaude
authored
Restore rule-based flattening of array dependent variables (#101)
#98 rewrote `chain_flatten_array_variables` to avoid a QA ignore for `Chain` and `Prewalk`. The rewrite keyed its replacement map on the array operation alone, so every `u[i]` overwrote the previous entry and all indices collapsed onto the last one, and it built replacements with `Symbolics.variable(...; T = symtype(name))`, which yields an array-valued variable rather than a callable. The resulting `Arr` reached `get_ops`, so discretizing any `PDESystem` written against `@variables (u(..))[1:N]` failed with `MethodError: operation(::Symbolics.Arr{Num, 1})`. The ignore was avoidable without touching this code: `Chain` and `Prewalk` are public in `SymbolicUtils.Rewriters`, which owns them, and were only non-public via the parent `SymbolicUtils`. Import them from the owner and restore the previous implementation, which shipped in 0.1.30. Fixes #100 Claude-Session: https://claude.ai/code/session_0117aJvytDT3y2jMi5Ng6Q5m Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 97c9de5 commit 01a7e79

3 files changed

Lines changed: 36 additions & 22 deletions

File tree

src/PDEBase.jl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ using SciMLBase: AbstractDiscretization, AbstractDiscretizationMetadata, Nonline
1414
ODEFunction, ODEProblem
1515
import SciMLPublic: @public
1616
using Symbolics: @variables, Differential, Equation, Num, unwrap
17-
using SymbolicUtils: arguments, getmetadata, iscall, operation, substitute, unwrap_const
17+
using SymbolicUtils: @rule, arguments, getmetadata, iscall, operation, substitute,
18+
unwrap_const
19+
using SymbolicUtils.Rewriters: Chain, Prewalk
1820
using SymbolicIndexingInterface: is_time_dependent
1921
using TermInterface: maketerm, metadata
2022

src/make_pdesys_compatible.jl

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,36 +26,19 @@ function _replace_ops(term, op_map)
2626
return term
2727
end
2828

29-
function _replace_indexed_array_variables(term, replacements)
30-
term = safe_unwrap(term)
31-
iscall(term) || return term
32-
33-
op = operation(term)
34-
args = arguments(term)
35-
if op === getindex
36-
array = first(args)
37-
if iscall(array)
38-
replacement = get(replacements, operation(array), nothing)
39-
replacement === nothing || return replacement(arguments(array)...)
40-
end
41-
end
42-
43-
new_args = _replace_indexed_array_variables.(args, Ref(replacements))
44-
return all(isequal.(args, new_args)) ? term : maketerm(typeof(term), op, new_args, metadata(term))
45-
end
46-
4729
function chain_flatten_array_variables(dvs)
48-
replacements = Dict()
30+
rs = []
4931
for dv in dvs
5032
dv = safe_unwrap(dv)
5133
if isequal(operation(dv), getindex)
5234
name = operation(arguments(dv)[1])
5335
idxs = arguments(dv)[2:end]
5436
fullname = Symbol(string(name) * "_" * string(idxs))
55-
replacements[name] = Symbolics.variable(fullname; T = SymbolicUtils.symtype(name))
37+
newop = (@variables $fullname(..))[1]
38+
push!(rs, @rule getindex($(name)(~~a), idxs...) => newop(~a...))
5639
end
5740
end
58-
return isempty(replacements) ? identity : term -> _replace_indexed_array_variables(term, replacements)
41+
return isempty(rs) ? identity : Prewalk(Chain(rs))
5942
end
6043

6144
function apply_lhs_rhs(f, eqs)

test/array_variables.jl

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
using PDEBase
2+
using ModelingToolkit
3+
using Symbolics
4+
using SymbolicUtils
5+
using Test
6+
7+
@testset "Flattening array dependent variables" begin
8+
N = 3
9+
@parameters x
10+
@variables (u(..))[1:N]
11+
12+
dvs = collect([u(x)[i] for i in 1:N])
13+
ch = PDEBase.chain_flatten_array_variables(dvs)
14+
flattened = map(dv -> ch(PDEBase.safe_unwrap(dv)), dvs)
15+
16+
# Each `u[i]` needs its own replacement; keying only on `u` collapses them all.
17+
@test length(unique(string.(flattened))) == N
18+
19+
for f in flattened
20+
# The replacement stays a call in the independent variables so downstream
21+
# `operation`/`arguments` keep working. An `Arr` here means the index was
22+
# never resolved away.
23+
@test !(f isa Symbolics.Arr)
24+
@test iscall(f)
25+
@test isequal(only(arguments(f)), PDEBase.safe_unwrap(x))
26+
end
27+
28+
@test PDEBase.chain_flatten_array_variables([]) === identity
29+
end

0 commit comments

Comments
 (0)