Repository navigation
Fix nested ForwardDiff tag mismatch - #3406
AdityaPandeyCN wants to merge 1 commit into
Conversation
Signed-off-by: AdityaPandeyCN <adityapand3y666@gmail.com>
| function (ff::SciMLBase.UJacobianWrapper{true})(du1, uprev::AbstractArray{<:ForwardDiff.Dual}) | ||
| p = ff.p | ||
| if p isa AbstractArray && eltype(p) <: ForwardDiff.Dual | ||
| DualU = eltype(uprev) | ||
| if !(eltype(p) <: DualU) | ||
| hasfield(typeof(ff.f), :f) && getfield(ff.f, :f) isa FunctionWrappersWrappers.FunctionWrappersWrapper && return ff.f(du1, uprev, p, ff.t) |
There was a problem hiding this comment.
No need to do it like this, it should just be in the solve dispatch?
There was a problem hiding this comment.
I tried it but the DiffEqBase solve path can't promote p because the nested dual type doesn't exist yet at that point , it's created later by Rosenbrock's JacobianConfig. Promoting early caused triple nesting.
There was a problem hiding this comment.
It's incorrect to just unwrap the functionwrapper though, that means the wrong call is getting compiled, so it needs to be handled at the source of the issue.
There was a problem hiding this comment.
Makes sense. I'll move the fix to jacobian! in derivative_wrappers.jl , widen p once before DI.jacobian! runs, same approach as #3389. or you could reopen that.
|
Closing: this is fixed at the root in ForwardDiff itself by JuliaDiff/ForwardDiff.jl#820 (containment-proof tag ordering, successor of ForwardDiff#807), so the downstream workaround is no longer needed. Thanks for the investigation on this — the tagcount-inversion diagnosis and the ≺ suggestion in the ForwardDiff#724 thread fed directly into the final fix. |
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Fix for SciML/OrdinaryDiffEq.jl#3381. When NonlinearSolve computes a Jacobian over an ODE solve,
penters asDual{NLTag}and Rosenbrock's internal ForwardDiff seedsuasDual{OrdEqTag, Dual{NLTag}, CS}. The user function f(du, u, p, t) multiplies across tag levels, producing the wrong outer tag and crashing onsetindex!.