Skip to content
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

Updated parameter file and example notebook for lithium plating on composite electrodes #4691

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

mohammedasher
Copy link

Description

Updated the chen2020_composite to include lithium plating parameters. Added an example notebook "lithium-plating-composite" for demonstrating plating on composite electrodes.

Addresses issue 4273 - #4273

Fixes # (issue)

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.

  • [x ] New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • [x ] Bug fix (non-breaking change which fixes an issue)

Key checklist:

  • [ x] No style issues: $ 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)
  • [ x] All tests pass: $ python -m pytest (or $ nox -s tests)
  • [ x] The documentation builds: $ python -m pytest --doctest-plus src (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ nox -s quick.

Further checks:

  • [x ] Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 23.07692% with 20 lines in your changes missing coverage. Please review.

Project coverage is 99.13%. Comparing base (864388f) to head (914e491).
Report is 8 commits behind head on develop.

Files with missing lines Patch % Lines
...input/parameters/lithium_ion/Chen2020_composite.py 23.07% 20 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4691      +/-   ##
===========================================
- Coverage    99.21%   99.13%   -0.09%     
===========================================
  Files          302      302              
  Lines        22880    22909      +29     
===========================================
+ Hits         22701    22710       +9     
- Misses         179      199      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@kratman kratman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some questions. Also, please do not remove the requested reviewers. The automatically requested reviewers are the people that wanted to be responsible for reviewing these files

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look like it should have been included

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simulating plating on composite electrodes requires these parameters. Currently, there are no definitive research papers providing these parameters for composite electrodes. As a placeholder, we are using parameters from OKane2022 with the assumption that the parameters for both phases are identical.

Please note that these values are provisional and should be updated as and when new research becomes available. While this is not ideal, these parameters are essential to enable simulation and further development on degradation mechanisms in composite electrodes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was referring to the file marked as copy. It looked like something mistakenly included

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, this is mistakenly included.

Comment on lines +507 to +530
# lithium plating
# Plating parameters referred from OKane2022
"Lithium metal partial molar volume [m3.mol-1]": 1.3e-05,
"Primary: Lithium plating kinetic rate constant [m.s-1]": 1e-09,
"Primary: Exchange-current density for plating [A.m-2]"
"": graphite_plating_exchange_current_density_OKane2020,
"Primary: Exchange-current density for stripping [A.m-2]"
"": graphite_stripping_exchange_current_density_OKane2020,
"Primary: Initial plated lithium concentration [mol.m-3]": 0.0,
"Primary: Typical plated lithium concentration [mol.m-3]": 1000.0,
"Primary: Lithium plating transfer coefficient": 0.65,
"Primary: Dead lithium decay constant [s-1]": 1e-06,
"Primary: Dead lithium decay rate [s-1]"
"": graphite_SEI_limited_dead_lithium_OKane2022,
"Secondary: Lithium plating kinetic rate constant [m.s-1]": 1e-09,
"Secondary: Exchange-current density for plating [A.m-2]"
"": silicon_plating_exchange_current_density_OKane2020,
"Secondary: Exchange-current density for stripping [A.m-2]"
"": silicon_stripping_exchange_current_density_OKane2020,
"Secondary: Initial plated lithium concentration [mol.m-3]": 0.0,
"Secondary: Typical plated lithium concentration [mol.m-3]": 1000.0,
"Secondary: Lithium plating transfer coefficient": 0.65,
"Secondary: Dead lithium decay constant [s-1]": 1e-06,
"Secondary: Dead lithium decay rate [s-1]"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these parameters from the same paper as before?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These parameters are not from Chen2020. Currently, no definitive research papers are providing these parameters for composite electrodes. As a placeholder, we use parameters from OKane2022 assuming that the parameters for both phases are identical.

Please note that these values are provisional and should be updated as and when new research becomes available. While this is not ideal, these parameters are essential to enable simulation and further development of degradation mechanisms in composite electrodes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer to keep named parameter sets true to what is in the paper (I know this isn't true in all cases, but we should aim for this). It would be better to add the extra parameters directly in the example notebook rather than modify the Chen 2020 set.

Copy link
Contributor

@DrSOKane DrSOKane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mohammedasher, I agree with @rtimms suggestion of leaving the parameter sets unchanged and defining the parameters in the notebook itself. This works even with function parameters, because you can define the function in the notebook. I appreciate this makes for a long notebook, but it will be a useful teaching aid for other advanced PyBaMM users who need to add physics to parameter sets that don't have them. Now that mechanics is working as well, should we widen the scope of this example to include mechanics as well? Or we could keep it plating-focused and add a parametric study, to see what effect having different plating parameters on each phase will have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants