Fix Downgrade (Core) and Aqua/JET QA failures#58
Draft
ChrisRackauckas-Claude wants to merge 1 commit into
Draft
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>
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.
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