Fix out-of-place DAEProblem solve; make the test suite actually run#93
Merged
ChrisRackauckas merged 1 commit intoJun 5, 2026
Merged
Conversation
The out-of-place branch of the SciMLBase common-interface `solve` wired the residual as a 2-arg ODE RHS `f = (t, u) -> prob.f(u, p, t)`, but a DAEProblem's out-of-place residual is 4-arg `(du, u, p, t)` and the solver calls it with `(t, u, du)`. Every out-of-place DAE therefore errored. Mirror the in-place wiring: `f = (t, u, du) -> prob.f(du, u, p, t)`. `test/common.jl` only exercised the in-place `prob_dae_resrob`, so the out-of-place path was untested. Add a non-MTK out-of-place index-1 DAE regression test (u1' = -u1, 0 = u1 - u2; exact u1 = u2 = exp(-t)) that errors without the fix. runtests.jl used `GROUP = get(ENV, "GROUP", "all")`, but the downgrade/CI workflow exports `GROUP=""` (empty, not unset), so `get` returned "" and both the `core` and `QA` guards were false: CI ran zero tests yet reported success. Coerce an empty GROUP to "all" so the suite actually runs. With the suite now running, three pre-existing `@allocated == 0` assertions in alloc_tests.jl surfaced as failures. They are measurement artifacts, not real allocations: measured at (non-const) global testset scope, a `Float64` return is boxed and escape analysis cannot elide a returned view's `SubArray` wrapper. Measured inside a function barrier (typed-local args, direct call) the three functions allocate 0. The barrier keeps the strict `== 0` guard — a genuine view->copy regression still allocates (a fresh array) and is still caught. Verified locally on Julia 1.10: full suite passes (Pkg.test exit 0), including common interface 4/4, ModelingToolkit DAE Initialization 46/46, QA 20/20. 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.
Please ignore until reviewed by @ChrisRackauckas. Draft.
Source fix: out-of-place
DAEProblemwas brokenThe out-of-place branch of the common-interface
solve(src/common.jl) wired the residual as a 2-arg ODE RHS:But an out-of-place
DAEProblem's residual is 4-arg(du, u, p, t), and the solver calls the residual with(t, u, du). So every out-of-place DAE errored (MethodError, the 2-arg lambda can't take the solver's 3 args). Mirror the in-place wiring (F! = (out, t, u, du) -> prob.f(out, du, u, p, t)):test/common.jlonly exercised the in-placeprob_dae_resrob, so this path had no coverage. Added a non-MTK out-of-place index-1 DAE regression test (u1' = -u1,0 = u1 - u2; exactu1 = u2 = exp(-t)) that errors without the fix.Test infrastructure: the suite ran zero tests in CI
runtests.jlusedGROUP = get(ENV, "GROUP", "all"), but the CI/downgrade workflow exportsGROUP=""(empty, not unset).getreturns"", so both thecoreandQAguards were false — CI ran zero tests yet reported success. Coerced an emptyGROUPto"all".Alloc tests: fix a measurement artifact (not a loosen)
Once the suite actually ran, three
@allocated == 0assertions inalloc_tests.jlsurfaced as failures (dassl_norm,get_history_t!,get_history_y!). These are measurement artifacts of measuring@allocatedat non-const global testset scope: aFloat64return gets boxed (16 B) and escape analysis can't elide a returned view'sSubArraywrapper (48 B). Measured inside a function barrier (typed-local args, direct call) all three allocate 0. The== 0guard is preserved — a real view→copy regression still allocates a fresh array (≥80 B) inside the barrier and is still caught.Verification (local, Julia 1.10,
Pkg.test)🤖 Generated with Claude Code