fix(zygote ext): handle NamedTuple cotangents in vofa_u_adjoint (EnsembleSolution end-time grads)#623
Closed
ChrisRackauckas-Claude wants to merge 1 commit into
Conversation
04cca47 to
d5029ea
Compare
`vofa_u_adjoint` rewrapped the per-element cotangents of an `AbstractVectorOfArray`/`AbstractDiffEqArray` in a `VectorOfArray`/`DiffEqArray`. For an `EnsembleSolution` whose elements are solution objects, differentiating only a scalar field (e.g. `sol.t[end]`) makes Zygote produce structural `NamedTuple` cotangents per trajectory. These have no `ndims`, so the reconstruction threw `MethodError: no method matching ndims(::NamedTuple)`. Detect non-array cotangents and pass them through as a plain vector instead. This extends the earlier `ZeroTangent`/`AbstractZero` handling (SciML#606) to the structural-tangent case. Note: this is a local robustness fix only; it does NOT fix SciML/DifferentialEquations.jl#1149. The cotangent then fails downstream in SciMLBase's `EnsembleSolution_adjoint`, and SciMLSensitivity never consumes a `t` cotangent (verified: single-solve `sol.t[end]` loss silently returns zero gradient under GaussAdjoint while ForwardDiff gives the true nonzero one). See the PR description for full verification details. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
d5029ea to
c9fd4a7
Compare
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.
What this PR is (and is not)
This is a local robustness fix in the Zygote extension:
vofa_u_adjointmust not force structural (NamedTuple) cotangents into aVectorOfArray/DiffEqArray— that reconstruction callsndims(::NamedTuple), which has no method. Structural per-element cotangents now pass through as a plain vector.This does NOT fix SciML/DifferentialEquations.jl#1149. End-to-end verification (below) shows the failure merely moves downstream, and the underlying capability (reverse-mode gradients w.r.t.
sol.t, e.g.terminate!halting times) does not exist in the adjoint path at all.Original symptom
Zygote through an
EnsembleSolutionwhen the loss reads only a scalar field per trajectory:Each per-trajectory cotangent is a structural
NamedTuple((u = nothing, t = OneElement(...), …)), not an array, because onlysol.t[end]is differentiated. This is the structural-tangent sibling of theZeroTangentcase fixed in #606 (same function).End-to-end verification of the full issue MRE (Julia 1.11.9, OrdinaryDiffEq v7.1.1, SciMLBase v3.31.0, SciMLSensitivity v7.113.0, Zygote v0.7.11)
With this patch, the
ndimscrash is gone and theNamedTuplecotangents (carrying the realt::ChainRules.OneElementseed) flow through. The failure then relocates to SciMLBase:(the generic
p̄::AbstractArray{T,N}method assumesN ≥ 2; aVector{NamedTuple}matches it withN = 1sinceNamedTupleis not<:AbstractArray).More importantly, fixing that plumbing would produce silently wrong gradients.
Δ.tis consumed nowhere in SciMLSensitivity — the backpass only readsΔ.u. Verified on a minimal single-trajectory case (no ensemble, no RAT in the loop),loss(θ) = sol.t[end]withGaussAdjoint(ReverseDiffVJP())+terminate!:So the pre-existing state for the non-ensemble version of this loss is a silent zero gradient, which is arguably a worse bug than the crash reported in the issue.
Consequences
EnsembleSolutionend times when usingterminate!callbacks DifferentialEquations.jl#1149 belongs in SciMLSensitivity: either implement event-time adjoint corrections (IFT at the callback root, per arXiv 2011.03902), or at minimum throw an informative error when a nonzerotcotangent reaches an adjoint sensealg instead of silently dropping it.EnsembleSolution_adjointneeds a method for vectors of structural tangents (guarded by the same "don't silently dropt" consideration).Test
Added to
test/AD/adjoints.jl, mirroring the #606 test:NamedTuplecotangents throughvofa_u_adjointfor bothVectorOfArrayandDiffEqArray, plus a mixedNamedTuple/nothing/ZeroTangentcase. Verified locally: the new test reproduces the exactndims(::NamedTuple)error on unmodified source and passes with the fix; fullAD/adjoints.jlsuite passes.🤖 Generated with Claude Code