Fix downstream test sources for OrdinaryDiffEq 7#594
Conversation
The compat bump in the previous commit gets `activate_downstream_env` past `Pkg.develop`, but the downstream test sources still don't run because the OrdinaryDiffEq 7 resolution was previously masking three issues: - `Rosenbrock23(autodiff = false)` is no longer supported; OrdinaryDiffEq 7 rejects the `Bool` form and asks for an `ADType`. Use `AutoFiniteDiff()` (already reachable through `using OrdinaryDiffEq`). - `Rodas5` is no longer re-exported from `OrdinaryDiffEq`; pull it in via `using OrdinaryDiffEqRosenbrock` and add the package as a dep. - `AP[...]` shorthand requires `RecursiveArrayToolsShorthandConstructors` in scope; runtests.jl `Pkg.develop`s the package but never `using`-s it, so the downstream test files silently never had it. - `SciMLBase.successful_retcode(...)` requires `SciMLBase` as a bare binding, which `using OrdinaryDiffEq` doesn't provide. Switch to the unqualified `successful_retcode` (also already exported through OrdinaryDiffEq), avoiding the need to add SciMLBase as a direct dep. Verified locally that the ODE Solve / Events / Measurements and Units / TrackerExt safetestsets all run to completion after the fix. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
…test MTK 11 reorders ODESystem unknowns ([y, x] in this case, not [x, y]), so the hardcoded propertynames in the test/downstream/symbol_indexing.jl Tables interface check no longer match what `propertynames(row)` and `Tables.columnnames(row)` return on the solution rows. The data columns in `Array(sol_ts)'` already follow MTK's unknowns ordering, so deriving the names list from `unknowns(lv)` keeps names and data in sync and is robust to future MTK reorderings. Restores 357/358 assertions in the `DiffEqArray Indexing Tests` testset on Julia 1/lts/pre. The remaining 1-test error is in the array-symbolic indexing test at line 103 (`sol[x .+ [y, 2y, 3y]] ≈ ...`) — passes standalone, errors only inside `@safetestset`'s isolated module; this looks like a separate Symbolics/SII module-isolation interaction that needs its own investigation. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
|
Update: pushed an additional fix for MTK 11 reorders After this commit, The remaining 1-test error is at line 103 ( |
`sol[arr_sym, :]` should be equivalent to `sol[arr_sym]` — both ask for
the full timeseries of the symbolic indices. Currently the `:`-suffixed
form errors with a broadcast shape mismatch inside SII's `GetStateIndex`
when the underlying index resolves to a `Vector{Int}` (i.e. when the
symbol is array-valued like `x(t)[1:3]`):
DimensionMismatch: arrays could not be broadcast to a common size:
a has axes Base.OneTo(N) and b has axes Base.OneTo(K)
(SII's `(gsi::GetStateIndex)(::Timeseries, prob, i)` broadcasts `gsi.idx`
against `state_values(prob, i)` without wrapping it in a 1-tuple, so a
length-N timeseries broadcast against a length-K idx vector errors. A
companion fix is being submitted upstream.)
Short-circuiting `A[sym, :]` to `A[sym]` here both restores the intended
semantics and avoids the broken SII codepath.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
|
Update: pushed commit `39c3708` with a RAT-side workaround for the remaining `sol[x, :]` failure. Root causeThe error was `DimensionMismatch` inside `SymbolicIndexingInterface.GetStateIndex(::Timeseries, prob, i)` at `SII/src/state_indexing.jl:43`: Fix split
Verified``` |
Note
Please ignore this PR until reviewed by @ChrisRackauckas.
Summary
Follow-up to #593. That PR fixed the resolver, but the downstream test sources still don't run because the OrdinaryDiffEq 6→7 transition broke several call sites that the old resolver was masking. This PR makes the actual safetestsets pass.
Changes
test/downstream/odesolve.jl:Rosenbrock23(autodiff = false)→Rosenbrock23(autodiff = AutoFiniteDiff()). OrdinaryDiffEq 7 rejectsBoolfor that kwarg and asks for anADType.using OrdinaryDiffEqRosenbrock(Rodas5is no longer re-exported fromOrdinaryDiffEq).using RecursiveArrayToolsShorthandConstructors(needed for theAP[...]syntax introduced in 334bd95; runtests.jlPkg.develops the package but neverusing-s it).SciMLBase.successful_retcode(sol)→ baresuccessful_retcode(sol).successful_retcodeis reachable throughusing OrdinaryDiffEq;SciMLBaseas a bare name is not. Avoidingusing SciMLBasekeepsSciMLBase/ADTypesout of[deps]— adding them as direct deps would unresolvably conflict with RAT 4.3's transitive SciMLBase 3 requirement.test/downstream/downstream_events.jl:using RecursiveArrayToolsShorthandConstructors(sameAP[...]reason).test/downstream/Project.toml:OrdinaryDiffEqRosenbrock = "1, 2"andRecursiveArrayToolsShorthandConstructors.Local verification
After this branch:
All four
Downstream-group safetestsets thatruntests.jlexercises run to completion. TheBrokentotals are pre-existing@test_brokenmarkers, not new failures.Out of scope
test/downstream/symbol_indexing.jl(SymbolicIndexingInterfacegroup) still has many failures from MTK 11 / SII indexing API changes — its own investigation.test/downstream/adjoints.jlremains commented out inruntests.jlper the pre-existing TODO.Test plan
ODE Solve Testssafetestset passes locally (4 pass, 1 pre-existing broken)Event Tests with ArrayPartitionsafetestset runs without errorMeasurements and Unitssafetestset passes (1 pre-existing broken)TrackerExtsafetestset passes (1 pass)Tests - Downstream - Julia 1/lts/pre