Accumulation for ODEProblem #1036
Conversation
ext/SciMLBaseZygoteExt.jl
Outdated
| error() | ||
| dx = (; Zygote.nt_nothing(x)..., pair(Val(f), Δ, x)...) | ||
| (_project(x, dx), nothing) | ||
| else | ||
| dx = Zygote.grad_mut(__context__, x) | ||
| dx[] = (; dx[]..., pair(Val(f), Zygote.accum(getfield(dx[], f), Δ))...) | ||
| return (dx,nothing) |
There was a problem hiding this comment.
What's the point of this block if the first line is error()?
There was a problem hiding this comment.
Removed in a03bb02
| end | ||
| Zygote.unwrap(val), back | ||
| end | ||
| Zygote.accum(::Tuple{}, ::NamedTuple{}) = () |
There was a problem hiding this comment.
Isn't this type piracy?
There was a problem hiding this comment.
yeah... FluxML/Zygote.jl#1574 is definitely the safer bet.
Why are problem types mutable? Do they need to be?
src/initialization.jl
Outdated
| if initdata.initializeprobmap !== nothing | ||
| u0 = initdata.initializeprobmap(choose_branch(nlsol)) | ||
| u02 = initdata.initializeprobmap(nlsol2) |
There was a problem hiding this comment.
This doesn't make sense - we solve initialization, but calculate the new u0 from the unsolved problem?
There was a problem hiding this comment.
This has contains some of the changes we had made previously to resolve the error. It is possible to remove the changes to initialization.jl, just the Zygote patch should be sufficient.
|
Closing in favour of FluxML/Zygote.jl#1574 |
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Sister PR for FluxML/Zygote.jl#1574 which would have much narrower dispatch for
getfield.