GetStateIndex: wrap idx so it broadcasts as scalar across timeseries#157
Merged
ChrisRackauckas merged 2 commits intoMay 18, 2026
Merged
Conversation
`(gsi::GetStateIndex)(::Timeseries, prob, i)` previously did
getindex.(state_values(prob, i), gsi.idx)
which works when `gsi.idx` is scalar (broadcasts as length-1 against the
time axis), but errors when `gsi.idx` is array-valued — e.g. a
`Vector{Int}` produced when the user requested an array-symbolic like
`x(t)[1:3]` — because then the broadcast tries to zip the length-N
timeseries against the length-K index vector and errors:
DimensionMismatch: arrays could not be broadcast to a common size:
a has axes Base.OneTo(N) and b has axes Base.OneTo(K)
The no-`i` overload at line 38 already handles this correctly by
wrapping `gsi.idx` in a 1-tuple. Mirror that here so an array-valued
`idx` is always broadcast as a scalar across the time axis.
Reproduces in RecursiveArrayTools.jl downstream tests with MTK 11 +
OrdinaryDiffEq 7:
sol[x(t)[1:3], :]
# ERROR: DimensionMismatch (10 vs 3)
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
5 tasks
The previous tests for `getsym(sys, [:x, :y])(sol, ::Colon)` only exercise
the `MultipleGetters` indexer path — `getsym` builds one `GetStateIndex`
per symbol, never a single `GetStateIndex{Vector{Int}}`. The buggy
codepath fires when downstream packages (e.g. ModelingToolkit) bind an
array-valued symbolic like `x(t)[1:3]` to a single contiguous set of
state indices and construct `GetStateIndex(Vector{Int})` directly. This
test constructs that shape manually and exercises each non-`Int`/-`CartesianIndex`
`i` overload (`:`, `1:N`, `eachindex`, `BitVector`) so a future
regression would be caught here.
Reproduces a `DimensionMismatch: arrays could not be broadcast to a
common size: a has axes Base.OneTo(4) and b has axes Base.OneTo(2)`
without the `(gsi.idx,)` wrap.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Please ignore this PR until reviewed by @ChrisRackauckas.
Summary
(gsi::GetStateIndex)(::Timeseries, prob, i)(at `src/state_indexing.jl:43`) currently does:```julia
getindex.(state_values(prob, i), gsi.idx)
```
This works when `gsi.idx` is a scalar Int (broadcasts as length-1 against the time axis), but errors when `gsi.idx` is array-valued — e.g. a `Vector{Int}` produced when the symbolic is array-typed like `x(t)[1:3]` — because the broadcast then tries to zip the length-N timeseries against the length-K index vector:
```
DimensionMismatch: arrays could not be broadcast to a common size:
a has axes Base.OneTo(N) and b has axes Base.OneTo(K)
```
The companion no-`i` overload (line 38) already handles this with a 1-tuple wrap:
```julia
function (gsi::GetStateIndex)(::Timeseries, prob)
return getindex.(state_values(prob), (gsi.idx,)) # wrapped
end
```
This PR mirrors that wrap on the with-`i` overload so array-valued `idx` is always broadcast as a scalar across the time axis.
Reproduction
Triggered by `sol[x(t)[1:3], :]` in the RecursiveArrayTools.jl downstream tests with MTK 11 + OrdinaryDiffEq 7. After fix, the call returns the expected `Vector{Vector{Float64}}` matching `sol[x(t)[1:3]]`.
Side note
The same issue is being worked around on the RecursiveArrayTools.jl side in SciML/RecursiveArrayTools.jl#594 by short-circuiting `A[sym, :] → A[sym]` for symbolic indices; once this SII fix lands and gets released, that workaround can be removed.
Test plan