Skip to content

Forward-shift whole-array references in shift_discrete_system - #162

Closed
greekera1000 wants to merge 1 commit into
JuliaComputing:mainfrom
greekera1000:fix-whole-array-shift
Closed

greekera1000 wants to merge 1 commit into
JuliaComputing:mainfrom
greekera1000:fix-whole-array-shift

Conversation

@greekera1000

Copy link
Copy Markdown

Fixes SciML/ModelingToolkit.jl#5169.

Problem

When a discrete partition uses an array variable as a whole at a previous tick
(e.g. ud ~ kp * combine(xd(k - 1)) with a registered combine(::AbstractVector)),
the compiled partition's observed equation referenced xdₜ₋₂ instead of xdₜ₋₁,
a variable that is neither an unknown nor a parameter, so codegen failed with
UndefVarError.

Cause

shift_discrete_system collects variables with search_variables!, which descends
through Shift and finds the whole array xd. It then skips anything not in
fullvars, but fullvars only holds scalarized elements (xd[i],
Shift(-1)(xd)[i]), never the whole array. So the elements were shifted forward
while the whole-array reference Shift(-1)(xd) inside combine(...) was left at -1.
When ud later became observed, backshift_expr applied another Shift(-1),
giving Shift(-2)(xd) → xdₜ₋₂.

Elementwise use (xd(k-1)[1]) and scalars were unaffected because those terms are
themselves in fullvars.

Fix

Add is_scalarized_in: an array-shaped variable counts as present if all its scalar
elements are in fullvars, so it gets an entry in discmap and is shifted
consistently with its elements.

Testing

Ran the reproducer from the issue with this branch:

  • unknowns: [(xdₜ₋₁(t))[2], (xdₜ₋₁(t))[1]]
  • observed: ud(t) ~ combine(xdₜ₋₁(t))*kp (previously xdₜ₋₂)

disclosure:The root-cause analysis and pr description were done with the help of Claude
I reviewed the change and verified it locally by running the reproducer from the issue.

@baggepinnen

Copy link
Copy Markdown
Contributor

Did you see #161 ?

@greekera1000

Copy link
Copy Markdown
Author

sorry , I hadn't seen #161. It's the same root cause and fix, and yours already includes tests, so I'll close this one in favor of it. For what it's worth, I independently verified locally that the reproducer from SciML/ModelingToolkit.jl#5169 gives xdₜ₋₁ in the observed equation with this approach.

@greekera1000
greekera1000 deleted the fix-whole-array-shift branch September 21, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A shift applied to a whole array variable in a clock partition refers to an undeclared variable

2 participants