Fix Downgrade (Core) and Aqua/JET QA failures#58
Conversation
Downgrade (Core): raise SteadyStateDiffEq compat floor to 2.5. The old
"1, 2" floor let the downgrade resolver pick a SteadyStateDiffEq whose
solve.jl references SciMLBase.AbstractNonlinearTerminationMode against a
SciMLBase too old to define it, failing precompilation with
`UndefVarError: AbstractNonlinearTerminationMode not defined`. 2.5 forces a
SciMLBase floor (resolved 2.99) that defines the symbol. Also add the
missing SparseArrays compat entry (Aqua deps_compat) and raise the
DiffEqBase floor to 6.140 for downgrade resolvability.
QA / Aqua method-ambiguity + unbound-type-parameter: rewrite the
`pairedindices(::DefaultIndexHandler{N}, ::NTuple{N,T}, ...)` methods using
`Tuple{T, Vararg{T}}` (so T is always bound; NTuple{0,T} === Tuple{} left T
free) and add an explicit zero-dimensional method, removing both the
Number/AbstractVector ambiguity at NTuple{0} and the unbound M/T params in
the dimension-mismatch fallback.
QA / JET: qualify the NullParameters default argument as
`DiffEqBase.NullParameters()` in build_rhs.jl / build_rhs_ss.jl
(NullParameters is not exported from DiffEqBase, so the bare reference was an
undefined-binding error). This also clears the cascade of JET
`iterate(::Nothing)` reports that stemmed from the ambiguous/unbound
pairedindices methods.
Verified locally:
- Downgrade Core on Julia 1.10 (lts) with =floor-rewritten compat: telegraph
17/17, feedbackloop 12/12, birthdeath2D 18/18, tests passed.
- Core on Julia 1.12 ("1"): 17/17, 12/12, 18/18, tests passed.
- Aqua ambiguities/unbound/deps_compat: pass. JET.test_package: pass.
Remaining QA undefined-exports failure (CartesianGridReJ, infimum, iscall,
params, supremum) is inherited via `@reexport using Catalyst` and is an
upstream Catalyst export bug (Catalyst exports CartesianGridReJ but
JumpProcesses defines CartesianGridRej; the others come from MTK/Symbolics
re-exports). Not fixable from within FiniteStateProjection without disabling
the check; needs an upstream Catalyst release.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
86f1fdc to
865891b
Compare
|
Rebuilt this branch against current |
|
Pushed a follow-up to remove the downgraded steady-state flake in Local validation after the change:
|
Summary
Fixes the Downgrade (Core) check and the Aqua method-ambiguity / unbound-type-parameter / deps-compat and JET sub-checks of the QA jobs on
main. All fixes were reproduced and verified locally on the failing Julia versions before committing.1. Downgrade (Core) —
SteadyStateDiffEqcompat floorThe downgrade job failed precompiling
SteadyStateDiffEqwithUndefVarError: AbstractNonlinearTerminationMode not defined(itssolve.jl:70referencesSciMLBase.AbstractNonlinearTerminationMode). The old floorSteadyStateDiffEq = "1, 2"let the downgrade resolver pick aSteadyStateDiffEqpaired with aSciMLBasetoo old to define that symbol. Raised toSteadyStateDiffEq = "2.5", which forces aSciMLBasefloor (resolved 2.99.0) that defines it. Also added the missingSparseArrays = "1"compat (fixes the Aqua deps-compat failure) and raised theDiffEqBasefloor to6.140for downgrade resolvability.2. QA / Aqua — method ambiguity + unbound type parameters
pairedindices(::DefaultIndexHandler{N}, ::NTuple{N,T}, ::CartesianIndex{N})had aT<:Number/T<:AbstractVectorpair that was ambiguous atN == 0(NTuple{0,T} === Tuple{}leavesTfree) and anAbstractVectorfallback with unboundM/T. Rewrote the two methods usingTuple{T, Vararg{T}}(soTis always bound) plus an explicit zero-dimensional method, and dropped the unused type params on the dimension-mismatch fallback.3. QA / JET —
NullParametersNullParametersis not exported fromDiffEqBase, so the bare default-arg references inbuild_rhs.jl/build_rhs_ss.jlwere undefined bindings. Qualified asDiffEqBase.NullParameters(). This also clears the JETiterate(::Nothing)cascade that stemmed from the ambiguous/unboundpairedindicesmethods.Verification (run locally)
=floor-rewritten compat: telegraph 17/17, feedbackloop 12/12, birthdeath2D 18/18, tests passed. (CI: Downgrade (Core) is now green.)1): telegraph 17/17, feedbackloop 12/12, birthdeath2D 18/18, tests passed.test_ambiguities/test_unbound_args(1/1) /test_deps_compat(4/4), and JETtest_package(...; target_defined_modules=true)(1/1): all pass on Julia 1.12 against the package project..jlfiles (runic --checkclean).Remaining QA failure (upstream Catalyst — not fixable here)
The Aqua
test_undefined_exportssub-check is still red. The undefined exports —CartesianGridReJ,infimum,iscall,params,supremum— are all inherited via@reexport using Catalyst; FiniteStateProjection's own exports (FSPSystem,DefaultIndexHandler,SteadyState) are all defined.This is conclusively an upstream Catalyst bug, not an FSP defect. Catalyst itself fails the same Aqua check: running the equivalent of
Aqua.test_undefined_exportsagainstCatalystdirectly reports[:CartesianGridReJ, :infimum, :params, :supremum]as undefined exports of Catalyst. FSP inherits those four plusiscall(the extra one is a@reexportdouble-hop:iscallisusing-imported into Catalyst from TermInterface, so it isisdefined(Catalyst, :iscall)but is not re-bound in FSP).CartesianGridReJ(uppercaseJ) is a literal export typo in Catalyst —src/Catalyst.jlexportsCartesianGridReJwhile the type defined in JumpProcesses (and used everywhere else in Catalyst) isCartesianGridRej(lowercasej). The uppercase-Jsymbol is defined in no module in the dependency tree, so it cannot be madeisdefinedin FSP without inventing a fake binding. The typo is present on Catalyst 15.0.11 (the latest 15.x) and on Catalyst master.infimum,supremum,paramsareusing-imported-then-exported by Catalyst from ModelingToolkit / Symbolics / DomainSets and do not propagate transitive bindings through@reexport using Catalyst.Bumping to Catalyst 16.2 does not fix this (it yields a different set of undefined re-exports and additionally breaks FSP's Core tests — see the dependabot Catalyst-16.2 PR, whose Core jobs are red). Making this check green requires an upstream Catalyst fix (at minimum, fixing the
CartesianGridReJ->CartesianGridRejexport typo). It cannot be fixed inside FiniteStateProjection without disabling/excluding the check or fabricating a binding.Note on the
Core (julia pre)CI jobOn a later CI run this PR shows 3 failing asserts in
birthdeath2D.jl(steady-state marginal/permutation tests,atol=1e-4) on thejulia prechannel (1.13.0-rc1). This is not a regression from this PR: the same job was green onmain, the resolved dependency set is byte-identical between the two runs (sameSteadyStateDiffEq 2.11.0,NonlinearSolve 4.16.0,Catalyst 15.0.11,SciMLBase 2.153.1), and locally on1.13.0-rc1with that exact resolved environmentbirthdeath2Dpasses 18/18 (reproduced twice). The failures are borderline-tolerance numerical drift in theSSRootfindsteady-state solve on the Julia release candidate, not a deterministic FSP change.This PR makes Downgrade fully green and reduces QA from 5 failing Aqua/JET sub-checks to the single upstream-blocked
test_undefined_exportscheck.Please ignore until reviewed by @ChrisRackauckas.
🤖 Generated with Claude Code