From c0dfda2c70d5da3e44a94f8f36555da87b4445b0 Mon Sep 17 00:00:00 2001 From: ChrisRackauckas-Claude Date: Thu, 4 Jun 2026 22:29:00 -0400 Subject: [PATCH] Fix out-of-place DAEProblem solve and make the test suite actually run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/common.jl | 5 +++-- test/alloc_tests.jl | 30 +++++++++++++++--------------- test/common.jl | 16 ++++++++++++++++ test/runtests.jl | 2 +- 4 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/common.jl b/src/common.jl index 0a5f546..ab6ba22 100644 --- a/src/common.jl +++ b/src/common.jl @@ -96,8 +96,9 @@ function solve( verbose = alg.verbose ) else - # Out-of-place path (unchanged, backward compatible) - f = (t, u) -> prob.f(u, p, t) + # Out-of-place path: residual must be 3-arg (t, u, du) for the DAE solver, + # mirroring the in-place F!(out, t, u, du) wiring above (was 2-arg ODE form). + f = (t, u, du) -> prob.f(du, u, p, t) ts, timeseries, dus = dasslSolve( f, u0, tspan, diff --git a/test/alloc_tests.jl b/test/alloc_tests.jl index 37e62ea..09f8d43 100644 --- a/test/alloc_tests.jl +++ b/test/alloc_tests.jl @@ -5,15 +5,22 @@ using DASSL using AllocCheck using Test +# `@allocated expr` measured directly in a testset body runs at (non-const) global +# scope, which inflates the count above the function's true cost: a bits return value +# (e.g. a `Float64`) gets boxed, and escape analysis can't elide the `SubArray` wrapper +# of a returned view. Measuring inside a specialized function barrier (typed-local args, +# direct call) reflects real call-site behavior. These stay strict zero-allocation +# guards: a genuine data copy still allocates inside the barrier (e.g. `x[1:n]` returns a +# fresh array), so a view->copy regression is still caught. +norm_allocs(v, wt) = (DASSL.dassl_norm(v, wt); @allocated DASSL.dassl_norm(v, wt)) +hist_t_allocs(cache, ord) = (DASSL.get_history_t!(cache, ord); @allocated DASSL.get_history_t!(cache, ord)) +hist_y_allocs(cache, ord) = (DASSL.get_history_y!(cache, ord); @allocated DASSL.get_history_y!(cache, ord)) + @testset "Allocation Tests" begin @testset "dassl_norm - zero allocations" begin v = [1.0, 2.0, 3.0] wt = [0.5, 0.5, 0.5] - # Warmup - DASSL.dassl_norm(v, wt) - # Test allocations - allocs = @allocated DASSL.dassl_norm(v, wt) - @test allocs == 0 + @test norm_allocs(v, wt) == 0 end @testset "_all_steps_equal - zero allocations" begin @@ -163,16 +170,9 @@ using Test DASSL.push_history!(cache, 0.1, y0 .+ 0.1, zeros(2)) DASSL.push_history!(cache, 0.2, y0 .+ 0.2, zeros(2)) - # Warmup - DASSL.get_history_t!(cache, 2) - DASSL.get_history_y!(cache, 2) - - # Test allocations - views into pre-allocated buffers should be zero-alloc - allocs = @allocated DASSL.get_history_t!(cache, 2) - @test allocs == 0 - - allocs = @allocated DASSL.get_history_y!(cache, 2) - @test allocs == 0 + # Views into pre-allocated buffers should be zero-alloc (no data copy). + @test hist_t_allocs(cache, 2) == 0 + @test hist_y_allocs(cache, 2) == 0 end @testset "In-place stepper! allocation check" begin diff --git a/test/common.jl b/test/common.jl index f230a58..14ed84c 100644 --- a/test/common.jl +++ b/test/common.jl @@ -6,3 +6,19 @@ prob = prob_dae_resrob sol = solve(prob, dassl()) sol = solve(prob, dassl(), abstol = 1.0e-1, reltol = 1.0e-2) + +# Out-of-place DAEProblem path: prob.f is the 4-arg residual (du, u, p, t). Regression +# test for the OOP wrapper, which previously called it as a 2-arg ODE RHS (t, u) and +# errored for every out-of-place DAE. Index-1 DAE: u1' = -u1, 0 = u1 - u2, so the exact +# solution is u1(t) = u2(t) = exp(-t). +@testset "Out-of-place DAEProblem" begin + dae_oop(du, u, p, t) = [du[1] + u[1], u[1] - u[2]] + u0 = [1.0, 1.0] + du0 = [-1.0, -1.0] + oop_prob = DAEProblem(dae_oop, du0, u0, (0.0, 1.0)) + @test !DiffEqBase.isinplace(oop_prob) # confirm the OOP branch is exercised + oop_sol = solve(oop_prob, dassl(), abstol = 1.0e-8, reltol = 1.0e-8) + @test oop_sol.retcode == ReturnCode.Success + @test isapprox(oop_sol.u[end][1], exp(-1); atol = 1.0e-5) + @test isapprox(oop_sol.u[end][2], exp(-1); atol = 1.0e-5) # algebraic constraint held +end diff --git a/test/runtests.jl b/test/runtests.jl index 06c937b..2d8867c 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1,7 +1,7 @@ using DASSL, Test using LinearAlgebra: diagm, I -const GROUP = get(ENV, "GROUP", "all") +const GROUP = let g = get(ENV, "GROUP", "all"); isempty(g) ? "all" : g end if GROUP == "all" || GROUP == "core" @testset "Testing maxorder" begin