Fix Adjoint Sensitivities #237
Conversation
mjohnson541
force-pushed
the
fix_adjoint_sens
branch
2 times, most recently
from
8bbe639
to
4a4f16c
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #237 +/- ##
==========================================
- Coverage 49.82% 49.70% -0.13%
==========================================
Files 31 31
Lines 8086 8138 +52
==========================================
+ Hits 4029 4045 +16
- Misses 4057 4093 +36 ☔ View full report in Codecov by Sentry. |
mjohnson541
force-pushed
the
fix_adjoint_sens
branch
from
43de642
to
e80bb70
Compare
|
Ultimately, the nasty error message was a red herring. However, I did resolve it by changing the typing for getadjointsensitivities. The real issue I suspect is that in SciMLSensitivity.jl at too tight of absolute tolerances the CVODE_BDF adjoint solve fails returning an integration that only accounts for the early trajectory of the adjoint solve resulting in nonsense sensitivities. CVODE_BDF does give a warning when this happens, but I think it's one that we're used to ignoring. Reducing the absolute tolerances a little resolves this issue for all tests. |
| @@ -459,8 +459,8 @@ By default uses the InterpolatingAdjoint algorithm with vector Jacobian products | |||
| this assumes no changes in code branching during simulation, if that were to become no longer true, the Tracker | |||
| based alternative algorithm is slower, but avoids this concern. | |||
| """ | |||
| function getadjointsensitivities(bsol::Q, target::String, solver::W; sensalg::W2=InterpolatingAdjoint(autojacvec=ReverseDiffVJP(false)), | |||
| abstol::Float64=1e-6, reltol::Float64=1e-3, normalize=true, kwargs...) where {Q,W,W2} | |||
| function getadjointsensitivities(bsol::Simulation, target::String, solver; sensalg=InterpolatingAdjoint(autojacvec=ReverseDiffVJP(false)), | |||
There was a problem hiding this comment.
Why do we remove typing for W and W2?
| @@ -561,7 +562,7 @@ function getadjointsensitivities(bsol::Simulation, target::String, solver; sensa | |||
| dpadj ./= bsol.sol(bsol.sol.t[end])[ind] | |||
| end | |||
| end | |||
| return dpadj | |||
| return dpadj::LinearAlgebra.Adjoint{Float64, Vector{Float64}} | |||
There was a problem hiding this comment.
I'm surprised that this is needed
Issue #235 seems to suggest the issue may be resolved by updating to SciMLSensitivities.jl.