-
-
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
Partially reversible lithium plating #2043
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.
See comments. Also, please add tests for partially reversible plating
pybamm/input/parameters/lithium_ion/lithium_platings/okane2022_Li_plating/parameters.csv
Show resolved
Hide resolved
tests/unit/test_parameters/test_parameter_sets/test_OKane2020.py
Outdated
Show resolved
Hide resolved
…xchange current densities
…UpdateParameters
…nt' for partially reversible plating
Codecov Report
@@ Coverage Diff @@
## develop #2043 +/- ##
========================================
Coverage 99.38% 99.38%
========================================
Files 348 352 +4
Lines 19234 19289 +55
========================================
+ Hits 19115 19170 +55
Misses 119 119
Continue to review full report at Codecov.
|
For some reason, merging UpdateParameters into this branch fixed the error stopping that from being merged into develop, so that pull request is now closed and the same changes proposed as part of this one. |
…s; Chen2020_plating remains adjusted
…f that parameer is not defined
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 looks great now, thanks @DrSOKane !
.../parameters/lithium_ion/negative_electrodes/graphite_Chen2020/graphite_LGM50_ocp_Chen2020.py
Outdated
Show resolved
Hide resolved
Forgot to ask for a new line in CHANGELOG, can you do that? |
I have pushed the new line to my fork, but not sure if that will be pulled in or if I have to make a new request
…________________________________
From: Valentin Sulzer ***@***.***>
Sent: 16 June 2022 16:55
To: pybamm-team/PyBaMM ***@***.***>
Cc: O'Kane, Simon ***@***.***>; Mention ***@***.***>
Subject: Re: [pybamm-team/PyBaMM] Partially reversible lithium plating (PR #2043)
This email from ***@***.*** originates from outside Imperial. Do not click on links and attachments unless you recognise the sender. If you trust the sender, add them to your safe senders list<https://spam.ic.ac.uk/SpamConsole/Senders.aspx> to disable email stamping for this address.
Forgot to ask for a new line in CHANGELOG, can you do that?
—
Reply to this email directly, view it on GitHub<#2043 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKH3KYIWNCXH6ST2CHQ4NSTVPNE7PANCNFSM5USFA77A>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Description
The partially reversible lithium plating model proposed by O'Kane et al. (2022) is added to PyBaMM.
Fixes # 1807
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:
$ flake8
$ python run-tests.py --unit
test_symbol/test_visualize fails
$ cd docs
and then$ make clean; make html
You can run all three at once, using
$ python run-tests.py --quick
.Further checks: