-
-
Notifications
You must be signed in to change notification settings - Fork 546
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
Add functions of time in experiment step #4222
Add functions of time in experiment step #4222
Conversation
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.
Thanks, this is perfect, just need tests for coverage
@@ -2,6 +2,7 @@ | |||
|
|||
## Features | |||
|
|||
- Added functionality to pass in arbitrary functions of time as the argument for a (`pybamm.step`). ([#4222](https://github.com/pybamm-team/PyBaMM/pull/4222)) |
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.
This should be made into a new section as it won't be included in this release
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.
This is under Unreleased right now. Do you want me to move everything else under Unreleased into a new section?
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.
Yes - though let's check with release team @Saransh-cpp @agriyakhetarpal @kratman to see the best way to deal with this
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.
Since this PR is going into the develop
branch and is a feature addition, the merged commit here won't be cherry-picked into further release candidates – it will go straight into v24.9. I think it should be fine to leave as is. Ideally, cherry-picked bug fixes will carry their own CHANGELOG entries into the next release candidate or final release.
Since the release notes are still copied over manually, we'll have a chance there to check for unneeded entries
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4222 +/- ##
========================================
Coverage 99.55% 99.55%
========================================
Files 288 288
Lines 21856 21871 +15
========================================
+ Hits 21759 21774 +15
Misses 97 97 ☔ View full report in Codecov by Sentry. |
For steps that compute their charge/discharge direction ( |
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!
* time varying experiment steps * Update CHANGELOG.md * fix code coverage * rearrange function order * remove `is_drive_cycle` attribute * break out `record_tags` method --------- Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
Description
Adds functionality to pass in arbitrary functions of time as the argument for a
pybamm.step
Fixes #4187
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
) (I haven't gottenidaklu
installed so I'm not certain all tests pass)$ 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: