SciML / SciML/ProcessSimulator.jl
Track simple_cstr array bounds and stale sparsity failures
Nobody has claimed this yet.
- Dominant language
- Julia
- Stars
- 44
- Forks
- 8
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 6
Description
Summary
The clean main commit 60e9d241513b3b1375aaff08e1b8e0e179a08b36 fails the Core group's simple_cstr.jl test while constructing its ODEProblem. The immediate cause is scalar bounds metadata attached to array-valued variables in TPControlVolume; ModelingToolkit 11 requires each bound to have the same shape as the symbolic array.
Correcting those bounds exposes a second, independent ModelingToolkit bug: the retained tearing graph produces a 6×6 Jacobian sparsity pattern after compilation has reduced the final system and mass matrix to 5×5. That upstream fix is tracked in https://github.com/SciML/ModelingToolkit.jl/pull/5051.
Reproduction
GROUP=Core julia --project -e 'using Pkg; Pkg.test()'
Observed on the unmodified base:
ERROR: LoadError: AssertionError: !(symbolic_has_known_size(arrx)) || SU.shape(arrx) == SU.shape(b)
...
in expression starting at test/simple_cstr.jl:95
Test Summary: | Error Total
Simple CSTR | 1 1
A focused fail-before run against the proposed ModelingToolkit sparsity fix still produces the same assertion at simple_cstr.jl:95, confirming that the array-bounds change is independently required in ProcessSimulator.
Proposed ProcessSimulator fix
Use shaped bounds for the array variables:
ϱ:(zeros(N_ph), fill(Inf, N_ph))nᵢ:(zeros(N_ph, N_c), fill(Inf, N_ph, N_c))xᵢ:(zeros(N_ph, N_c), ones(N_ph, N_c))
The current test environment also needs to import QNDF directly from OrdinaryDiffEqBDF, because DifferentialEquations 8 no longer exports it. Sampling the solution on a fixed 0.1-second grid gives a stable peak-temperature regression (356.151 K at 2824.8 s) instead of depending on adaptive solver step locations.
With both the ProcessSimulator changes and the ModelingToolkit sparsity fix applied locally:
Core:
Public API Tests 8/8
Simple CSTR 3/3
Simple Steady State Test 4/4
QA: 20/20
History
The scalar array bounds originated in https://github.com/SciML/ProcessSimulator.jl/commit/b88132a6137e9af31a01b8a9b3bbad9a85f14ebf. They were accepted by ModelingToolkit 9.84, where indexing an array variable with scalar bounds returned (0, Inf). ProcessSimulator commit https://github.com/SciML/ProcessSimulator.jl/commit/1a02801ee0e0c3e494f99194e1b2889dea811eb6 first admitted ModelingToolkit 11, where the same standalone indexed-bounds reproducer fails the shape assertion.
A ProcessSimulator fix PR should wait until the upstream ModelingToolkit fix is merged and released; otherwise the package's Core test advances past bounds handling and fails at the independent 6×6/5×5 sparsity mismatch. The touched simple_cstr.jl also has unrelated pre-existing Runic indentation drift that should land through the repository's separate formatting PR rather than be mixed into this behavioral fix.
🤖 Investigated with Codex CLI 0.151.0 (model: unknown; session: local session ID 01a04fa1-cfe0-7260-b416-72fe8a15d17d).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Run GROUP=Core julia --project -e 'using Pkg; Pkg.test()' and inspect test/simple_cstr.jl around line 95, along with the TPControlVolume array-bound declarations. Apply the shaped bounds and use the direct QNDF import described in the issue, then validate the fixed-grid temperature regression and Core/QA tests after ModelingToolkit PR #5051 is released.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- julia
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100