Fix master CI: JET undefined-var, CSTR init over-determination, downgrade compat floors#52
Draft
ChrisRackauckas-Claude wants to merge 1 commit into
Draft
Conversation
…rade compat floors Three independent master failures on SciML/ProcessSimulator.jl: QA / JET (`local variable ΔHᵣ may be undefined`): `TPControlVolume` created the reaction variables `ΔnR`/`ΔHᵣ` inside a `reactive ? append!(vars, @variables ...) : nothing` ternary, so when `reactive == false` the symbolic variables were never bound, yet the equation list still references `ΔHᵣ`/`ΔnR` (guarded by `reactive ? ... : 0.0`). JET's `report_package` flagged the possibly-undefined local. Move the `@variables` block out of the ternary so the variables are always created and only appended to the system's variable list when `reactive`. `JET.report_package` now reports 0 errors and `run_qa` passes 12/12 on Julia 1.12. Core tests (`simple_cstr.jl`, all Julia versions): `Tmax = 297.0` instead of 356.149. The test pinned `cstr.cv.c2.n` and `outlet.m` in `u0`; these are fixed by the algebraic flow balances, so pinning them over-determines MTK's initialization (4 residuals, 0 free unknowns) and the least-squares fallback returns a degenerate steady state where no reaction occurs. Supply them as initialization `guesses` instead. The peak-temperature *time* assertion is relaxed from atol 1e-2 to 0.1 because the maximum sits on a flat plateau (< 0.002 K across the bracketing steps) so the discrete argmax time shifts by one solver step (~0.02 s) across Julia versions while `Tmax` stays identical to 6 figures; the physically meaningful `Tmax` assertion stays tight at 1e-3. Verified `Pass 2/2` on Julia 1.10 and 1.12. Downgrade (Julia 1.10): `UndefVarError: AbstractNonlinearTerminationMode not defined`. The compat floors `ModelingToolkit = "9"` / `DifferentialEquations = "7"` let downgrade-compat resolve an inconsistent SciML stack (NonlinearSolve 3.14 + DiffEqBase 6.174 + SteadyStateDiffEq 2.0, and BoundaryValueDiffEqFIRK 1.6 against RecursiveArrayTools 3.50). Raise the floors to `ModelingToolkit = "9.60"` (pins NonlinearSolve 4.x) and `DifferentialEquations = "7.16"` (pins a modern BVP/SteadyState stack). Raising lower bounds only narrows resolution. Verified the downgraded resolution precompiles and `simple_cstr.jl` passes 2/2 on Julia 1.10. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
2 participants
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.
Summary
Fixes the three independent failures on
mainCI (HEAD7cad441):QA / JET —
local variable ΔHᵣ may be undefined. InTPControlVolume(src/base/base_components.jl), the reaction variablesΔnR/ΔHᵣwere created with@variablesinside areactive ? append!(...) : nothingternary, so they were never bound whenreactive == false, while the equation list references them (guarded byreactive ? ... : 0.0). Moving the@variablesblock out of the ternary makes them always defined; they are only appended to the system's variable list whenreactive.JET.report_packagenow reports 0 errors andrun_qapasses 12/12 on Julia 1.12.Core tests (
test/simple_cstr.jl, all Julia versions) —Tmax = 297.0instead of356.149(no reaction). The test pinnedcstr.cv.c2.nandoutlet.minu0; these are fixed by the algebraic flow balances, so pinning them over-determines MTK initialization (4 residuals, 0 free unknowns) and the least-squares fallback yields a degenerate steady state. Supplied as initializationguessesinstead. The peak-time assertion is relaxed1e-2 → 0.1because the maximum sits on a flat plateau (<0.002 K across the bracketing steps), so the discrete argmax time shifts by one solver step (~0.02 s) across Julia versions whileTmaxis identical to 6 figures; the physically meaningfulTmaxassertion stays tight at1e-3. VerifiedPass 2/2on Julia 1.10 and 1.12.Downgrade (Julia 1.10) —
UndefVarError: AbstractNonlinearTerminationMode not defined. The floorsModelingToolkit = "9"/DifferentialEquations = "7"let downgrade-compat resolve an inconsistent SciML stack (NonlinearSolve 3.14 + DiffEqBase 6.174 + SteadyStateDiffEq 2.0; later, BoundaryValueDiffEqFIRK 1.6 vs RecursiveArrayTools 3.50). Raised toModelingToolkit = "9.60"(pins NonlinearSolve 4.x) andDifferentialEquations = "7.16"(pins a modern BVP/SteadyState stack). Raising lower bounds only narrows resolution. Verified the downgraded resolution precompiles andsimple_cstr.jlpasses 2/2 on Julia 1.10.Local verification
JET.report_package→ 0 reports;run_qa→Quality Assurance | 12 12;simple_cstr.jl→Pass 2/2.julia-downgrade-compat'sdowngrade.jl) precompiles cleanly with the new floors;simple_cstr.jl→Pass 2/2.Files Runic-formatted.
Please ignore until reviewed by @ChrisRackauckas