SciML / SciML/ProcessSimulator.jl

Track simple_cstr array bounds and stale sparsity failures

Open
#60 0 comments 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.