PyBaMM icon indicating copy to clipboard operation
PyBaMM copied to clipboard

Improvements to IDAKLU solver

Open martinjrobins opened this issue 3 years ago • 3 comments

Description

Many things still left to do on the IDAKLU solver, here is the copied list from #1863

  • [ ] we're allocating memory for return vectors in the solve, this could be done in solver setup
  • [x] we initialise ida_mem and everything every time, could do this once when solver is created, or only on the 1st solve etc
  • [x] create an options dict to pass to the solver, e.g. option to print solver stats, don't/do use jacobian
  • [x] add option to use iterative solvers
  • [x] add option to use other linear solvers in ida
  • [x] add option to use dense jacobian
  • [ ] add option to use banded solvers
  • [ ] benchmarks for all of the above solver options
  • [ ] investigate if we need to densify the rhs_algebraic casadi evaluation, could it return a sparse vector?
  • [ ] investigate if constructing a new casadi function for the residual (rather than doing it manually in c++) is faster
  • [ ] investigate if constructing a new casadi function for the sensitivity calculations (rather than doing it manually in c++) is faster
  • [ ] we calculate consistent initial conditions at the start of the idklu solve, so we might not need to do it in the BaseSolver setup
  • [x] Sundials v6 has a slightly different API using a context object, need to update code to support both this and version 5

Motivation

  1. The IDAKLU solver can be faster than casadi, so we want to expose more functionality from sundials
  2. Make the solver more efficient

Possible Implementation

No response

Additional context

No response

martinjrobins avatar Aug 05 '22 10:08 martinjrobins

"we're allocating memory for return vectors in the solve, this could be done in solver setup" Need to think more on this one, if we re-use this memory for subsequent solves will overwrite solution returned to user :(

martinjrobins avatar Aug 08 '22 12:08 martinjrobins

There are still scenarios where that's ok - e.g. parameter estimation - so it could be an optional feature?

valentinsulzer avatar Aug 08 '22 17:08 valentinsulzer

Been reading a bit more about Sundials. Super excited to see these changes when they're ready! For pack modeling it could be extremely useful

valentinsulzer avatar Oct 12 '22 18:10 valentinsulzer

Are we now in a place where we can make the IDAKLU solver the default if it's installed (and fall back to casadi otherwise)? Would be interesting to test/benchmark

valentinsulzer avatar Dec 13 '22 16:12 valentinsulzer

I think so, we just have to first add the idaklu solver to the benchmarks so we've got some evidence for making it the default, I've opened #2599 for this

martinjrobins avatar Jan 10 '23 08:01 martinjrobins