-
-
Notifications
You must be signed in to change notification settings - Fork 563
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
feat: add discrete time sum expression tree node #4501
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4501 +/- ##
========================================
Coverage 99.42% 99.42%
========================================
Files 297 299 +2
Lines 22685 22739 +54
========================================
+ Hits 22554 22608 +54
Misses 131 131 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks for also separating out and simplifying the ExplicitTimeIntegral
. The changelog needs updating as well.
* feat: add discrete time sum expression tree node pybamm-team#4485 * docs: fix math syntax in docstring * remove prints * test casadi solver as well * coverage * coverage * add to changelog and tidy solution test
Description
Adds two new subclasses to
pybamm.Symbol
:pybamm.DiscreteTimeData
which is an interpolant for holding time series datapybamm.DiscreteTimeSum
which is a unary operator representing a sum of its child evaluation over a set of data time points. Its child must have exactly onepybamm.DiscreteTimeData
object.When
pybamm.DiscreteTimeSum
is evaluated normally it returns the evaluation of its child. If it is evaluated as part of apybamm.Solution
then it interpolates this solution onto the data time points and sums these over the time axisThis also updates the evaluation of
pybamm.ExplicitTimeIntegral
. Previously this performed a trapezoidal integral over all the time points passed to the__call__
function ofpybamm.ProcessedVariable
. Now it performs the integral over the solution time points, which is more consistent with its definition and always returns the same value irrespective of the time values that the user passes to__call__
. In the future this integral should be updated to use the gradient information of the solution if available.Fixes #4485
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: