-
-
Notifications
You must be signed in to change notification settings - Fork 553
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
Fixed a bug where the SEI thickness decreased at some intervals when using the 'electron-migration limited' model. #3622
Conversation
…ing only negative contributions to eta_sei in the j_sei
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3622 +/- ##
========================================
Coverage 99.59% 99.59%
========================================
Files 258 258
Lines 20755 20755
========================================
Hits 20670 20670
Misses 85 85 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Eric G. Kratz <[email protected]>
I am happy with that, but been quite involved in the discussions with Kawa, so requesting other people to review it. |
We should add a CHANGELOG line about this, should we open a separate PR or should we revert this merge? |
@all-contributors please add @kawaMANMI for bug report and code |
I've put up a pull request to add @kawaMANMI! 🎉 |
@brosaplanella Sorry about that, I did not check for the change log. I would say just fix it in another PR. Reverting is messy |
…using the 'electron-migration limited' model. (pybamm-team#3622) * The 'electron-migration limited' model has been corrected by considering only negative contributions to eta_sei in the j_sei * Update pybamm/models/submodels/interface/sei/sei_growth.py Co-authored-by: Eric G. Kratz <[email protected]> * Logical condition (eta_inner < 0) used instead pybamm.EqualHeaviside for more clarity --------- Co-authored-by: Eric G. Kratz <[email protected]>
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
The SEI thickness decreased at some intervals when the 'electron-migration limited' model was used. It has been corrected by considering only negative contributions to eta_sei in the j_sei, using the Heaviside function.
Fixes #3613
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: