Fix GaussAdjoint on immutable (SVector) ODE state#1479
Conversation
`GaussAdjoint` (and `GaussKronrodAdjoint`) errored with `AssertionError: sensealg isa QuadratureAdjoint` on out-of-place ODEs with immutable state such as `StaticArrays.SVector`, even though the default automatic sensealg choice selects `GaussAdjoint` for such problems. Fixing the assertion exposed three further problems in the immutable-state path, all fixed here: 1. `ReverseLossCallback`'s immutable-state branches asserted `sensealg isa QuadratureAdjoint`. The branches are valid for any adjoint whose state is exactly `λ` with no parameter-gradient augmentation, which is what the existing `isq` flag encodes. Widen `isq` to cover `AbstractGAdjoint` (it previously missed `GaussKronrodAdjoint`) and assert `isq` instead. 2. The `IntegratingSumCallback`/`IntegratingGKSumCallback` integrand was always passed in the 4-arg mutating form `(out, u, t, integrator)`, but DiffEqCallbacks calls the 3-arg allocating form `(u, t, integrator)` when the adjoint problem is out-of-place. Pass the allocating form for immutable state. 3. The out-of-place `split_states(u, t, S)` ignored checkpointing: it interpolated the (possibly non-dense) forward solution directly and never refreshed `checkpoint_sol`/`GaussInt.sol`, so the quadrature integrand extrapolated a stale checkpoint solution and produced silently wrong `dG/dp` whenever checkpointing was active (e.g. a `saveat` forward solution). Factor the checkpoint-refresh logic into `Gaussupdate_checkpoint_sol!` and make the out-of-place `split_states` checkpoint-aware. 4. The quadrature accumulation buffers were derived from `tunables` via `allocate_zeros`/`allocate_vjp`, which preserve staticness, so static parameters (e.g. `SVector` `p`) gave immutable buffers that DiffEqCallbacks then tried to mutate. Add a `mutable_buffer` helper and use it for the accumulation buffers and `GaussIntegrand` outputs. The mutable-state paths are unchanged: `mutable_buffer` is the identity for mutable arrays, the 4-arg integrand form is kept for mutable state, and the mutating `split_states` is untouched. Adds regression tests to `test/adjoint_oop.jl` comparing GaussAdjoint / GaussKronrodAdjoint (EnzymeVJP and ZygoteVJP) discrete and continuous adjoints on an `SVector` Lotka-Volterra problem against ForwardDiff references, including the checkpointing path, plus a through-`solve` Zygote gradient with the default automatic vjp choice. Fixes SciML#1461. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback on SciML#1479: the mutable counterpart of an immutable array is what similar already provides (an MVector for an SVector), so derive the quadrature accumulation buffers from a zeroed similar rather than collect, keeping static sizing and avoiding the heap Vector. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Re: On whether the allocating out-of-place integrand path costs anything — timed it (10-state Integrand level ( End-to-end Where it does matter is cheap integrands. With a trivial 100-element integrand on an out-of-place So I opened SciML/DiffEqCallbacks.jl#313 adding an opt-in (Two pre-existing gaps noticed while benchmarking, not touched here: |
…umCallback (#313) The integrating sum callbacks chose the integrand calling convention from the problem's in-placeness: out-of-place problems always used the allocating 3-arg `integrand_func(u, t, integrator)` form, even when an `integrand_prototype` was supplied. But the integrand output is often parameter-shaped (e.g. the dG/dp quadrature in adjoint methods), so it can be a mutable buffer even when the state is immutable (e.g. SVector) — and the allocating form then forces an output allocation on every quadrature node of every step. Add an `integrand_inplace::Union{Nothing, Bool} = nothing` keyword: - `nothing` (default) keeps the existing behavior (in-place form for in-place problems with a prototype, allocating form otherwise), - `true` forces the in-place 4-arg form into `integrand_cache` regardless of problem in-placeness (requires an `integrand_prototype`), - `false` forces the allocating form. Benchmark (OOP SVector ODE, 10 states, cheap 100-element integrand, Tsit5 over [0, 10], abstol=reltol=1e-10, full solve): - allocating 3-arg integrand: 196.8 μs (865 allocations: 380.70 KiB) - in-place 4-arg integrand: 123.3 μs (253 allocations: 103.38 KiB) For expensive integrands (e.g. AD-based vjps) the difference is negligible since the integrand itself dominates. Motivated by SciML/SciMLSensitivity.jl#1479 (GaussAdjoint on immutable SVector state), which currently has to use the allocating form for immutable state. Co-authored-by: Chris Rackauckas (Claude) <accounts@chrisrackauckas.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
DiffEqCallbacks 4.18 added `integrand_inplace` to the integrating sum
callbacks, so the Gauss quadrature integrand can use the in-place 4-arg form
unconditionally — the dG/dp buffer is parameter-shaped and mutable regardless
of state immutability. Drop the allocating 3-arg closure and bump the
DiffEqCallbacks compat to 4.18.
Also remove the avoidable allocations/work in the out-of-place adjoint hot
paths:
- The out-of-place `_vecjacobian` (EnzymeVJP and ZygoteVJP) differentiated
with respect to both `u` and `p` on every call, but the Gauss/Quadrature
adjoint rhs calls it with `dgrad === nothing` on every solver stage, so the
parameter gradient was computed and thrown away. Mark `p` constant
(Enzyme.Const / close over it for Zygote) when `dgrad === nothing`, and
skip the extra primal evaluation `f(y, p, t)` when `dy === nothing`
(EnzymeVJP only; Zygote gets the primal from the pullback).
- `vec_pjac!` (both gauss_adjoint.jl and quadrature_adjoint.jl) for
out-of-place functions used `Enzyme.gradient`, allocating a fresh gradient
vector and result tuple per quadrature node. When the output buffer type
matches the tunables (the common `Vector` parameter case), write the
gradient directly into `out` via `Enzyme.autodiff` with
`Duplicated(tunables, out)`; immutable (static) tunables keep the
`Enzyme.gradient` fallback.
Benchmark (10-state SVector ODE, 100 parameters, discrete-cost
adjoint_sensitivities, EnzymeVJP, seeded; identical dp checksums before and
after):
- GaussAdjoint: 35.86 ms / 549,423 allocs / 15.87 MiB
-> 31.37 ms / 462,095 allocs / 13.18 MiB
- QuadratureAdjoint: 42.90 ms / 651,695 allocs / 19.20 MiB
-> 37.68 ms / 564,475 allocs / 16.56 MiB
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Updated in dbfa6ee now that DiffEqCallbacks 4.18 is out: the Gauss integrand always uses the in-place 4-arg form via Also hunted down the remaining removable allocations in OOP mode (same commit):
Seeded A/B (10-state SVector ODE, 100 params, discrete-cost
Verified locally on the final tree: What's left allocates inside the Enzyme/Zygote runtimes per vjp call, not in our plumbing. Two further candidates noted but out of scope here: |
Important
This PR should be ignored until reviewed by @ChrisRackauckas.
Fixes #1461.
Problem
GaussAdjoint(the default sensealg choice for many out-of-place problems) errored withAssertionError: sensealg isa QuadratureAdjointinReverseLossCallbackwhen the ODE state is immutable (e.g. aStaticArrays.SVector). Fixing the assertion exposed three further bugs in the immutable-state path, all fixed here:src/adjoint_common.jl): the immutable-state branches ofReverseLossCallbackare valid for any adjoint whose integrator state is exactlyλwith no parameter-gradient augmentation — exactly what the existingisqflag encodes.isqis widened toAbstractGAdjoint(it previously missedGaussKronrodAdjoint) and the asserts now checkisq.src/gauss_adjoint.jl): DiffEqCallbacks'IntegratingSumCallback/IntegratingGKSumCallbackcall the integrand asintegrand_func(u, t, integrator)(allocating) when the adjoint problem is out-of-place, but the 4-arg mutating form was always passed, giving aMethodError. The allocating form is now passed for immutable state.split_states(src/gauss_adjoint.jl): the OOPsplit_statesignored checkpointing — it interpolated the (possibly non-dense) forward solution and never refreshedcheckpoint_sol/GaussInt.sol, so the quadrature integrand extrapolated a stale checkpoint solution and silently produced garbagedG/dpwhenever checkpointing was active (e.g. asaveatforward solution;ischeckpointing(::GaussAdjoint, sol) = checkpointing || !sol.dense). The checkpoint-refresh logic is factored intoGaussupdate_checkpoint_sol!and the OOPsplit_statesis now checkpoint-aware. (Caught by the new tests:du0looked fine whiledpwas wrong by orders of magnitude.)src/gauss_adjoint.jl,src/parameters_handling.jl): with static parameters (e.g.SVectorp), the quadrature accumulation buffers derived viaallocate_zeros/allocate_vjpwere themselves static, and DiffEqCallbacks'recursive_zero!then hitsetindex!(::SVector, ...). A newmutable_bufferhelper (identity for mutable arrays) makes the accumulation buffers mutable.The mutable-state paths are unchanged (all new branches reduce to the previous behavior for mutable state/parameters).
Verification (all run locally)
AssertionErroratsrc/adjoint_common.jl:753; after this PR the assertion no longer fires.SVectorLotka–Volterra,Zygote.gradientthroughsolvevsForwardDiff.gradient:GaussAdjoint(EnzymeVJP())relerr ≈ 5.5e-12GaussAdjoint(ZygoteVJP())relerr ≈ 5.4e-12GaussAdjoint()(automatic vjp) relerr ≈ 5.4e-12GaussKronrodAdjoint(EnzymeVJP())relerr ≈ 5.6e-12GaussAdjoint(ZygoteVJP())and with the default automatic sensealg choice, under both outerZygote.gradientand outerEnzyme.gradient(outer Enzyme ≡ outer Zygote exactly; both matchForwardDiffto Float32 tolerance, maxreldiff ≈ 1e-6).test/adjoint_oop.jlpasses in full locally (all pre-existing tests plus the new regression tests, which compare discrete/continuous adjoints — including the checkpointing path — against ForwardDiff references).test/gauss_zygote_inplace.jlpasses (mutable-path smoke test).Out of scope (pre-existing, shared with
QuadratureAdjoint)GaussAdjoint(autojacvec = EnzymeVJP())still fails on SimpleChains models, but for an unrelated upstream reason: Enzyme cannot differentiate SimpleChains'TurboDenseSIMD kernels (MethodError: from_tape_type(::Type{VecElement{Float32}})). Verified thatQuadratureAdjoint(EnzymeVJP())fails identically on the same MWE, so this is an Enzyme.jl/SimpleChains limitation, not an adjoint-path issue.EnzymeVJPon immutable state works fine for plain Julia rhs functions (tested).ReverseDiffVJPhas no out-of-place_vecjacobianmethod, soGaussAdjoint(ReverseDiffVJP())andQuadratureAdjoint(ReverseDiffVJP())both fail on immutable state with the sameMethodError(verified on master). This PR bringsGaussAdjointto parity withQuadratureAdjoint; the ReverseDiff gap is a separate issue.🤖 Generated with Claude Code