Fast Hermite interpolation and observables#4464
Merged
MarcBerliner merged 22 commits intopybamm-team:developfrom Oct 2, 2024
Merged
Fast Hermite interpolation and observables#4464MarcBerliner merged 22 commits intopybamm-team:developfrom
MarcBerliner merged 22 commits intopybamm-team:developfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
martinjrobins
requested changes
Oct 1, 2024
Contributor
martinjrobins
left a comment
There was a problem hiding this comment.
great stuff, thanks @MarcBerliner! I like the refactor of processed variable into subclasses. A few minor points below
kratman
reviewed
Oct 1, 2024
Contributor
There was a problem hiding this comment.
From my side it looks good, but lets wait for @martinjrobins to approve of the changes
martinjrobins
approved these changes
Oct 2, 2024
Contributor
martinjrobins
left a comment
There was a problem hiding this comment.
happy with this, thanks @MarcBerliner
6 tasks
pkalbhor
pushed a commit
to pkalbhor/PyBaMM
that referenced
this pull request
Nov 15, 2024
* fast observables * fix release and types * good * faster interp * `double` -> `realtype` * clean separation * good again * private members * cleanup * fix codecov, tests * naming * codecov * codacy, cse/expand * fix `try/except` * Update CHANGELOG.md * address comments * initialize `save_hermite` * fix codecov --------- Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.For simulations that save the adaptive time stepping states
y, the new solver optionhermite_interpolationuses the time derivativesypto obtain a more accurate interpolator. Previously, our post-solve interpolation involved:vfor the given raw datasol.all_tsandsol.all_ys(likev_raw = sol[v].data)v_rawonto new time pointst_new(likenp.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:
y_newmatrix by interpolatingsol.all_ysontot_newaccurately usingsol.all_yps, then observevat thet_newandy_newvalues. The Hermite interpolation API is justsol[v](t)sol[v].datato access thev_rawdata points at thesol.tpointsThese changes are most visible with very large datasets. Here are the new timings to get an accurate version of the
SPMwitht_new = np.linspace(t0, tf, 1000000):t_interp(old, Python): 10.6 sgiving a 25x speedup for this example.
Main changes in the code:
sol["Voltage [V]"].dataorsol["Voltage [V]"](t)) from Python to C++ProcessedVariableclass and adds subclasses likeProcessedVariable0D,ProcessedVariable1D, etc.all_ypsterm to theSolutionclasswarnargument from the interpolatorNotes:
yandypvalues. This means simulations withoutput_variablesort_interpvalues cannot be supported.ProcessedVariablethat will be cleaned up once we move to a standaloneIDAKLUpackageFixes #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.
Key checklist:
$ 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)$ python run-tests.py --all(or$ nox -s tests)$ 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: