Fix FullSpecialize initialization map buffer types - #5145
Draft
ChrisRackauckas-Claude wants to merge 2 commits into
Draft
Fix FullSpecialize initialization map buffer types#5145ChrisRackauckas-Claude wants to merge 2 commits into
ChrisRackauckas-Claude wants to merge 2 commits into
Conversation
`_static_initialization_buffer` promoted any mutable isbits buffer to an
`MVector`, including a plain `Vector`. The generated parameter map therefore
returned a different `MTKParameters` type than the problem carried, and
solve-time initialization assigns that straight back:
integrator.p = initializeprobpmap(prob, initsol)
which cannot convert. Any `ODEProblem{iip, FullSpecialize}` whose
initialization is solved at `solve` time rather than at construction threw
before its first step, a regression from SciML#5045.
Only produce static storage where the prototype is already static. The
allocation win then lands exactly where the problem opted into static storage
(a static `u0`/`p_constructor`, and DiffEqGPU's kernel path, which converts `p`
before the map runs) instead of being forced on every problem — which also
avoids building an `MVector` sized by the parameter count on large models.
The two assertions requiring `StaticVector` buffers from a `Vector`-backed
problem encoded the behaviour that breaks `solve`; they now require the map's
buffers to match the problem's own types, which is the actual contract. The
`static_constructor` cases still assert `isbits` results.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Agent-Harness: Claude Code 2.0.14
Agent-Model: claude-opus-5[1m]
Agent-Session: https://claude.ai/code/session_01GdSpCLd7NBZuuePJmcDzU7
Claude-Session: https://claude.ai/code/session_01GdSpCLd7NBZuuePJmcDzU7
The state map forced `iip ? MVector : SVector`, so
`ODEProblem{true, FullSpecialize}(sys, ...).u0` came back as an `MVector`
where it had been a `Vector` before SciML#5045 — a user-visible type change, and
one that breaks GPU ensembles outright, since a device array cannot have a
mutable element type:
JLArray only supports element types that are allocated inline.
MVector{1, Float64} is a mutable type
Build the buffer the way the default map path does and let `u0_constructor`
decide, matching the `p` handling in the previous commit. A caller that wants
static storage asks for it, and then gets an `SArray` — which is what a GPU
kernel needs anyway.
`_static_initialization_buffer` also has to fill its plain buffer explicitly:
StaticArrays overloads `collect` to preserve staticness, so
`collect(T, ::SVector)` returns a `SizedVector`. The parameter path passes a
`Tuple` and was unaffected; the state map passes an `SVector` and was not.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Agent-Harness: Claude Code 2.0.14
Agent-Model: claude-opus-5[1m]
Agent-Session: https://claude.ai/code/session_01GdSpCLd7NBZuuePJmcDzU7
Claude-Session: https://claude.ai/code/session_01GdSpCLd7NBZuuePJmcDzU7
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.
What changed and why
Two regressions from #5045, both from the
generated initialization maps rebuilding
u0/pin different container types than theproblem carries. Neither is GPU-specific — the first breaks a plain host
solve.1.
p(29981c03e2)._static_initialization_bufferpromoted any mutable isbitsbuffer to an
MVector, including a plainVector. Solve-time initialization assigns themap's result straight back:
which cannot convert. Any
ODEProblem{iip, FullSpecialize}whose initialization is solvedat
solvetime rather than at construction threw before its first step.2.
u0(80986a6532). The state map forcediip ? MVector : SVector, soODEProblem{true, FullSpecialize}(sys, …).u0came back as anMVectorwhere it had been aVector. That is a user-visible type change, and it breaks GPU ensembles outright, becausea device array cannot have a mutable element type:
Both now produce static storage only where the caller asked for it — a static
u0_constructor/p_constructor, or DiffEqGPU's kernel path, which convertspbefore themap runs. That is also where the allocation win was actually wanted; forcing it meant a
model with hundreds of tunables built an
MVectorthat size on every initialization.One subtlety worth flagging: the plain buffer is filled explicitly rather than with
collect, because StaticArrays overloadscollectto preserve staticness — socollect(T, ::SVector)hands back aSizedVector, not aVector. The parameter pathpasses a
Tupleand was unaffected; the state map passes anSVectorand was not.Verification
Julia 1.12.7. Both regressions bisect cleanly to #5045 — same script against the merge
commit's parent
0bba967daf:u0type, same comparison:New regression test, failing before / passing after (fix stashed and restored in one
session):
GROUP=Initialization, on the rebased branch:Exit 0, zero errors. The 12 broken are pre-existing
@test_brokens on master; none wereadded, removed, skipped or loosened. Runic,
typosover the diff andgit diff --checkare clean.
Test assertions I changed, and why
Three assertions from #5045 required
StaticVectorbuffers from aVector-backed problem.That is exactly the behaviour that breaks
solve, so they now require the map's buffers tomatch the problem's own types, which is the real contract. The
static_constructorcasesstill assert
isbitsresults, so the opt-in path is still pinned. Please push back if youwould rather keep forced-static and fix the integrator side instead.
This does narrow #5045's stated allocation result: the "784 → 80 bytes" row was measured on
a plain host problem, and that configuration now keeps
Vectorbuffers. The 0-bytestatic-constructor row and the GPU path are unaffected.
What I did not verify
Only
GROUP=Initializationwas run. Extensions, FMI, QA, SymbolicIndexingInterface,downstream and the docs build were not rerun for this change; CI is the arbiter. No GPU
hardware on this machine, so the
MVector-as-device-array-eltype failure is reproducedthrough
JLArrays, not CUDA.Ignore this draft until reviewed by @ChrisRackauckas.
🤖 Generated with Claude Code (model: claude-opus-5[1m])
https://claude.ai/code/session_01GdSpCLd7NBZuuePJmcDzU7