Towards a cleaner and more maintainable internals of NonlinearSolve.jl #203
Conversation
|
Can we setup a PDE benchmark before merging this so we know that the change is not a performance regression? It shouldn't be, but it would be good to know that the new sparse diff stuff is on par in performance with the setup we had before. |
|
You mean in SciMLBenchmarks? |
Codecov Report
@@ Coverage Diff @@
## master #203 +/- ##
==========================================
+ Coverage 94.42% 94.94% +0.52%
==========================================
Files 7 8 +1
Lines 699 732 +33
==========================================
+ Hits 660 695 +35
+ Misses 39 37 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
avik-pal
changed the title
avik-pal
changed the title
avik-pal
force-pushed
the
ap/cleanup
branch
from
0fbf311 to
a5ae510
Compare
avik-pal
force-pushed
the
ap/cleanup
branch
from
a5ae510 to
de8086c
Compare
avik-pal
changed the title
|
|
||
| autodiff = if iip && (ls.autodiff isa AutoZygote || ls.autodiff isa AutoSparseZygote) | ||
| @warn "Attempting to use Zygote.jl for linesearch on an in-place problem. Falling back to finite differencing." | ||
| AutoFiniteDiff() |
There was a problem hiding this comment.
why not fallback to ForwardDiff if it's being used elsewhere?
There was a problem hiding this comment.
I wanted to use VecJac, which doesn't have the forwarddiff yet (I think?). See https://github.com/JuliaDiff/SparseDiffTools.jl/blob/04810f5caea10633504ad44071f1a6b82cb9bb5a/src/differentiation/vecjac_products.jl#L51-L52
I can fix it upstream to just construct jacobian and then multiply, and then change the default here later
|
Needs SciML/DiffEqBase.jl#926 |
Fixes #195 #158 #140 #63
ADTypes-- constructs the ADType from the old argument style!TODOs:
resid_prototypeto properly constructfu(Add resid_prototype to NonlinearFunction SciMLBase.jl#490)Holding Off Merge till we have some upstream benchmarks