Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fast Hermite interpolation and observables #4464

Merged
merged 22 commits into from
Oct 2, 2024

Conversation

MarcBerliner
Copy link
Member

@MarcBerliner MarcBerliner commented Sep 24, 2024

Description

This PR adds Hermite interpolation to the IDAKLUSolver, which gives more accurate post-solve interpolation accuracy and improves performance by moving code from Python to C++. The high-frequency example below is 25x faster.

image

For simulations that save the adaptive time stepping states y, the new solver option hermite_interpolation uses the time derivatives yp to obtain a more accurate interpolator. Previously, our post-solve interpolation involved:

  1. Computing a variable v for the given raw data sol.all_ts and sol.all_ys (like v_raw = sol[v].data)
  2. Linear interpolating v_raw onto new time points t_new (like np.interp1d(t_new, sol.t, v_raw))

This was problematic because linear interpolation does not always provide an accurate solution (like in the example picture above). With the new Hermite interpolation, the two steps are combined into a single step that uses the state derivatives:

  • Create a y_new matrix by interpolating sol.all_ys onto t_new accurately using sol.all_yps, then observe v at the t_new and y_new values. The Hermite interpolation API is just sol[v](t)
  • We can still run sol[v].data to access the v_raw data points at the sol.t points

These changes are most visible with very large datasets. Here are the new timings to get an accurate version of the SPM with t_new = np.linspace(t0, tf, 1000000):

  • Dense t_interp (old, Python): 10.6 s
  • Hermite interp (new, C++): 0.421 s

giving a 25x speedup for this example.

Main changes in the code:

  • Moves the solution indexing (including sol["Voltage [V]"].data or sol["Voltage [V]"](t)) from Python to C++
  • Refactors the ProcessedVariable class and adds subclasses like ProcessedVariable0D, ProcessedVariable1D, etc.
  • Adds an all_yps term to the Solution class
  • Remove unused warn argument from the interpolator

Notes:

  • Hermite interpolation can only work if we have all the adaptive time stepping y and yp values. This means simulations with output_variables or t_interp values cannot be supported.
  • Sensitivity analyses are not currently using the C++ observable interface.
  • There are a few places in ProcessedVariable that will be cleaned up once we move to a standalone IDAKLU package

Fixes #4419

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

  • Optimization (back-end change that speeds up the code)

Key checklist:

  • No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
  • All tests pass: $ python run-tests.py --all (or $ nox -s tests)
  • The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.41%. Comparing base (08e5cf2) to head (4e708af).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4464      +/-   ##
===========================================
- Coverage    99.46%   99.41%   -0.05%     
===========================================
  Files          293      293              
  Lines        22332    22554     +222     
===========================================
+ Hits         22212    22423     +211     
- Misses         120      131      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kratman
kratman previously requested changes Oct 1, 2024
Copy link
Contributor

@kratman kratman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty minor changes

src/pybamm/solvers/c_solvers/idaklu/IDAKLUSolverOpenMP.hpp Outdated Show resolved Hide resolved
src/pybamm/solvers/solution.py Outdated Show resolved Hide resolved
src/pybamm/solvers/solution.py Outdated Show resolved Hide resolved
src/pybamm/solvers/solution.py Outdated Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Outdated Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Outdated Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Outdated Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Outdated Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Outdated Show resolved Hide resolved
Copy link
Contributor

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great stuff, thanks @MarcBerliner! I like the refactor of processed variable into subclasses. A few minor points below

src/pybamm/solvers/c_solvers/idaklu/IDAKLUSolverOpenMP.hpp Outdated Show resolved Hide resolved
src/pybamm/solvers/c_solvers/idaklu/IDAKLUSolverOpenMP.inl Outdated Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Outdated Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Outdated Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Outdated Show resolved Hide resolved
src/pybamm/solvers/processed_variable.py Outdated Show resolved Hide resolved
Copy link
Contributor

@kratman kratman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my side it looks good, but lets wait for @martinjrobins to approve of the changes

@kratman kratman self-requested a review October 1, 2024 21:09
@kratman kratman dismissed their stale review October 1, 2024 21:11

Changes were made

Copy link
Contributor

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy with this, thanks @MarcBerliner

@MarcBerliner MarcBerliner merged commit 444ecc1 into pybamm-team:develop Oct 2, 2024
24 of 26 checks passed
@MarcBerliner MarcBerliner deleted the fast-variable-observe branch October 2, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Post-solve Hermite interpolation in Sundials solvers
3 participants