SciML / OrdinaryDiffEq.jl

High performance ordinary differential equation (ODE) and differential-algebraic equation (DAE) solvers, including neural ordinary differential equations (neural ODEs) and scientific machine learning (SciML)
https://diffeq.sciml.ai/latest/
Other
523 stars 199 forks source link

Support NonlinearSolve.jl for the ODE nonlinear solver #2167

Closed oscardssmith closed 2 months ago

oscardssmith commented 3 months ago

This is WIP, but it mostly works (for in place ODE only), (and is a lot slower since it's missing all the fancy optimizations still).

oscardssmith commented 3 months ago

so the current status is that out of place solves now work (although something is going wrong since it is getting a lot of nonlinear solver convergence failures). In place is broken because _compute_rhs! isn't autodiff compatible, and no matter what I do, NonlinearSolve is trying to autodiff through it.

avik-pal commented 3 months ago

In place is broken because _compute_rhs! isn't autodiff compatible

How does the default newton in ordinarydiffeq handle this?

and is a lot slower since it's missing all the fancy optimizations still

you probably need the recompute_jacobian option here to mimic the jacobian reuse behavior https://github.com/SciML/NonlinearSolve.jl/blob/0dfc1358be16478a29e8940b5abca896d30ec9ff/src/core/generalized_first_order.jl#L204-L205

oscardssmith commented 3 months ago

with recompute_jacobian=false this actually is looking really good! I'm getting better stats than NLNewton (admitadly so far I've only been testing on f(u,p,t)=u. That said, I do seem to have somehow messed upNLNewton` in the process...

oscardssmith commented 3 months ago

it is worth mentioning that it is very possible that the better stats come from me not updating the stats properly. Still needs testing.

ChrisRackauckas commented 2 months ago

It's missing tests?

oscardssmith commented 2 months ago

I've added some tests that use this. I'm not sure how much needs testing.

ChrisRackauckas commented 2 months ago

Match a bit of what NLAnderson has done https://github.com/search?q=repo%3ASciML%2FOrdinaryDiffEq.jl+NLAnderson&type=code

avik-pal commented 2 months ago

Are we using the jacobian reuse here? or is that a plan for later

oscardssmith commented 2 months ago

I believe everything should now work. The last issues were that tstep and invγdt were being computed correctly, but the incorrect values for them were being passed into the nlp_params, leading to all sorts of nonsense.