-
Notifications
You must be signed in to change notification settings - Fork 24
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
Update default init_soc for DesignProblem #422
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #422 +/- ##
===========================================
+ Coverage 98.24% 98.52% +0.28%
===========================================
Files 46 46
Lines 2910 2923 +13
===========================================
+ Hits 2859 2880 +21
+ Misses 51 43 -8 ☔ 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 good - thanks! Just a few comments.
] | ||
|
||
if init_soc is None: | ||
if "Initial SoC" in model._parameter_set.keys(): |
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.
We should use the property for this private attribute instead of directly accessing it:
PyBOP/pybop/models/base_model.py
Line 662 in 8b09214
@property |
(throughout PR)
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.
@BradyPlanden shall I merge this one into #425 or |
Either is fine with me, although if it merges into |
* Update default init_soc * Update CHANGELOG.md * Update check_params * Add pybamm_model as default attribute * Ensure predict uses unprocessed_model * Remove unused store_optimised_parameters * Update parameters.initial_value * Use any Initial SoC from parameter_set * Update spm_electrode_design.ipynb * Move Changelog entry * Fix merge mistake * Add tests * Update description * Update check for ECM init_soc --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Description
Update the default value of the
init_soc
parameter ofDesignProblem
.Issue reference
Fixes #421
Review
Before you mark your PR as ready for review, please ensure that you've considered the following:
Type of change
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)$ nox -s tests
$ nox -s doctest
You can run integration tests, unit tests, and doctests together at once, using
$ nox -s quick
.Further checks:
Thank you for contributing to our project! Your efforts help us to deliver great software.