-
-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow BackSolveAdjoint to accept MTKParameters #1131
base: master
Are you sure you want to change the base?
Conversation
@@ -372,11 +372,6 @@ function DiffEqBase._concrete_solve_adjoint( | |||
saveat = eltype(prob.tspan)[], | |||
save_idxs = nothing, | |||
kwargs...) | |||
if !(sensealg isa GaussAdjoint) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If lifting this, then InterpolatingAdjoint and QuadartureAdjoint should also be tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, that's why I have marked this draft, so I can test, and if necessary add the check back with allowing BacksolveAdjoint through 😅
@@ -634,9 +634,19 @@ function DiffEqBase._concrete_solve_adjoint( | |||
|
|||
du0 = reshape(du0, size(u0)) | |||
|
|||
dp = p === nothing || p === SciMLBase.NullParameters() ? nothing : | |||
dp = p === nothing || p === DiffEqBase.NullParameters() ? nothing : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be SciMLBase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can move the refactoring in its own PR. It only happened because the last commit was for getting rid of some conflicts and the merge commit had that diff. Its not in the diff for this PR itself. We need to get some tests for the adjoint paths we expose in this PR.
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here. Closes #1130