Open brosaplanella opened 5 months ago
Not solver, but this is related to speed-up: https://github.com/pybamm-team/PyBaMM/issues/4058
This is my attempt at a roadmap for PyBaMM's solvers, comments / suggestions welcome
map
to parallise over inputsHere are my thoughts on improving the solver. Please let me know if you have comments or questions @martinjrobins and others.
In PyBaMM, the solver currently stops at all values in the t_eval
vector. It seems that we use t_eval
and set some dt
for a few distinct purposes:
period
experiment kwarg)dt
to force better solver convergence (take smaller time steps)These three reasons for setting a dt
are all valid, but stopping the simulation at all time steps can have a drastic impact on the adaptive time stepping and performance. For example, consider a full C/10 discharge with
t_eval = [0, 36000]
(i.e., no stops)t_eval = np.arange(0, 36060, 60)
(pybamm default)If we compare the solver stats,
165
vs b. 715
288
vs b. 823
28
vs b. 91
286
vs b. 821
25.5 ms
vs b. 97.1 ms
Even though we solve the same system, the dense t_eval
b. is nearly 4x slower! To address these issues, I propose the following changes that align the time-stepping options with Julia's differential equation solvers (see Output Control):
t
and y
determined by IDA's adaptive time stepping algorithm. This eliminates issues like the one above with the C/10 discharge. We can accurately post-interpolate the solution onto specific times with IDA's Hermite interpolator. This is a huge benefit for parameter estimation because we will always obtain the full solution accuracy regardless of the data collection frequency.t_eval
to match Julia's tstops
: "Denotes extra times that the time-stepping algorithm must step to. This should be used to help the solver deal with discontinuities and singularities, since stepping exactly at the time of the discontinuity will improve accuracy." With this option, drive cycles in 1. still worksaveat
: "Denotes specific times to save the solution at, during the solving phase. The solver will save at each of the timepoints in this array in the most efficient manner available to the solver." This addresses the data collection frequency in 2. without negatively affecting performance since it interpolates the solution with IDAGetDky()
.t_eval
for performance issues and encourage modifying appropriate solver options (rtol
, atol
, dt_max
, etc.), which addresses 3.yea, agree that it would be great to get rid of IDASetStopTime
! or at least reduce the number of times we use it. Would this play nicely with the casadi solver?
Would this play nicely with the casadi solver?
If we can access IDA's internal time steps via Casadi, I think we can do most of this stuff
Corollary from the PyBaMM developer meeting on 05/08/2024 as a part of PyBaMM running in WASM:
BUILD_IDAKLU
set to OFF
) in addition to platform-specific wheels, since pip
always chooses the most specific wheel available@agriyakhetarpal: FYI, I've compiled IDA with KLU using enscripten in the past, see this repo: https://github.com/martinjrobins/diffeq-runtime. However I agree that focusing on the casadi solver for now is the best approach since you have it already compiled and in Pyodide
Thanks for the resource, @martinjrobins! I see you've built a static lib for KLU and also used NO_LAPACK
– should work well. I'll tinker with the settings and maybe I can reduce the binary size a bit :)
To improve the speed of our ODE models, I'd like to add CVODE
from SUNDIALS to our suite of C solvers in addition to IDA
. I have a few questions about the implementation, and I'd like to hear your thoughts @jsbrittain, @martinjrobins, @pipliggins, and others.
IDA
and CVODE
. The solver code structure for both will be very similar, so this will help with code reuse. However, this will cause a headache for some of the existing PRs.CasadiSolver
automatically determines if the system is an ODE or a DAE and passes it to the appropriate solver. I think this approach also makes sense for our C solvers. If we do take this approach, then...IDAKLUSolver
name will no longer be accurate. Since we plan on streamlining the IDAKLU
installation process and changing the default away from CasadiSolver
, maybe we can also rename IDAKLUSolver
to SundialsSolver
or something. This might make the "new and improved default solver" announcement splashier (cc @valentinsulzer @rtimms).And just FYI, I don't plan on starting this work for at least a couple of weeks.
I'd like to continue making pybamm easier to work with for the parameter inference libraries like PyBOP (https://github.com/pybop-team/PyBOP). At the moment I think this includes:
Originally posted by @martinjrobins in https://github.com/pybamm-team/PyBaMM/issues/3839#issuecomment-1999464816