Added phase-dependent particle options to LAM

[parkec3]

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.

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.

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

Key checklist:

• 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)
• All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• The documentation builds: $ 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:

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

[codecov]

Codecov Report

Attention: Patch coverage is 98.14815% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.41%. Comparing base (fcbe756) to head (6a8e339).
Report is 198 commits behind head on develop.

Files with missing lines	Patch %	Lines
...els/submodels/particle_mechanics/base_mechanics.py	92.85%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4369      +/-   ##
===========================================
- Coverage    99.41%   99.41%   -0.01%     
===========================================
  Files          292      292              
  Lines        22213    22223      +10     
===========================================
+ Hits         22084    22093       +9     
- Misses         129      130       +1     

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

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[kratman]

Does this need to be put back or deleted?

[parkec3]

This needs to be deleted.

[DrSOKane]

Hi Caitlin,

This is a capability that PyBaMM has required for some time, so thank you very much for taking the time to add it! A couple of small changes are required, and I also have some deeper questions.

Required changes

• The reason the integration tests are failing is that the volume change functions you added take sto and c_s_max as arguments, when this parameter only takes sto as an argument. Removing c_s_max as an argument will fix this.
• The thickness changes are being added up in the wrong order. Phases should also be added before domains. Also, this code is not covered by any tests.

Broader questions

• I dispute your assertion that no parameter set exists for composite degradation. This paper from @mbonkile has one.
• Do you know why the model doesn't conserve lithium precisely?

Many thanks, Simon

[parkec3]

@DrSOKane thanks for your input. I updated the code as you suggested. For the degradation parameter set, I meant that there wasn't an available parameter set in pybamm to cover a composite electrode with LAM. It would be great to add the parameter set from the paper you referenced. I'm not sure what's behind the drop in accuracy for lithium conservation.

[DrSOKane]

Hi Caity, thanks for fixing these! We definitely need a Bonkile2024 parameter set, but that can be a separate PR. I'm still concerned about conservation, but there is precedent for allowing 9 digits, and so I will now approve the changes for merging.