MSMR model #3116

rtimms · 2023-07-06T18:03:49Z

Description

Implements the MSMR model from Baker & Verbrugge, 2018.

In particular, this PR adds a new "particle" model that solves for the potential within the particles. Then OCP is then this potential evaluated at the surface. In order to use this model, users must write down the OCV for each reaction (in the form of Eq 3), which are inverted to give the fraction of occupied sites (Eq 5). These are summed to give an explicit expression for the inverse of the OCV (Eq 6).

Also updates the electrode State of Health models to work with the MSMR formulation.

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 (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
The documentation builds: $ python run-tests.py --doctest

You can run unit and doctests together at once, using $ python run-tests.py --quick.

Further checks:

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

review-notebook-app · 2023-07-06T18:03:53Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

codecov · 2023-07-06T18:23:57Z

Codecov Report

Patch coverage: 95.46% and project coverage change: -0.16% ⚠️

Comparison is base (ec5383c) 99.70% compared to head (cb36b35) 99.55%.

❗ Current head cb36b35 differs from pull request most recent head 77a62c1. Consider uploading reports for the commit 77a62c1 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3116      +/-   ##
===========================================
- Coverage    99.70%   99.55%   -0.16%     
===========================================
  Files          248      253       +5     
  Lines        18899    19523     +624     
===========================================
+ Hits         18844    19436     +592     
- Misses          55       87      +32

Files Changed	Coverage Δ
pybamm/expression_tree/unary_operators.py	`100.00% <ø> (ø)`
pybamm/models/submodels/particle/base_particle.py	`100.00% <ø> (ø)`
...amm/models/full_battery_models/lithium_ion/msmr.py	`72.22% <72.22%> (ø)`
...odels/interface/open_circuit_potential/msmr_ocp.py	`88.88% <88.88%> (ø)`
pybamm/models/submodels/particle/msmr_diffusion.py	`93.24% <93.24%> (ø)`
...m/models/full_battery_models/base_battery_model.py	`99.33% <94.73%> (-0.21%)`	⬇️
...submodels/interface/kinetics/msmr_butler_volmer.py	`96.49% <96.49%> (ø)`
...s/full_battery_models/lithium_ion/electrode_soh.py	`98.88% <97.66%> (-1.12%)`	⬇️
pybamm/expression_tree/broadcasts.py	`100.00% <100.00%> (ø)`
...m/input/parameters/lithium_ion/MSMR_example_set.py	`100.00% <100.00%> (ø)`
... and 15 more

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

valentinsulzer

Looks good, will need some more testing for coverage. Let me know if you want me to review any equations in particular

valentinsulzer · 2023-07-06T19:21:06Z

pybamm/expression_tree/broadcasts.py

+        """
+        Override :meth:`pybamm.SpatialOperator.diff()` to reinstate behaviour of
+        :meth:`pybamm.Symbol.diff()`.
+        """


probably cleaner to just get rid of the NotImplementedError for spatial operators

rtimms · 2023-07-06T19:46:33Z

Thanks. Yep, tests and examples still to do but wanted another pair of eyes on the general implementation.

rtimms · 2023-07-10T15:25:10Z

The model and example are implemented. Remaining tasks:

Clean up eSOH (lots of if statements now). Could be tackled as a separate issue. It all works correctly as it is now, but the code isn't as clean as it could be.
Add helper functions for adding individual reaction variables to models (or explicitly give the number of reactions as a model option)
Doc rst

rtimms · 2023-08-11T14:57:02Z

To do:

Refomulate exchange current to use potential and lithiation extent (for numerical stability)
Make setting up options cleaner for half cells. At the moment the checks are set up such that you have to pass a tuple of options for the open circuit potential, particle and intercalation kinetics for a half cell even though the values in the li metal side are ignored (or don't make sense in the case of particle)

brosaplanella

Looks great, thanks Rob! Just a few comments, happy to merge once tests pass.

brosaplanella · 2023-09-01T11:45:48Z

docs/source/examples/notebooks/models/MSMR.ipynb

Picky comment, can you delete the empty cells at the end of the notebook?

brosaplanella · 2023-09-01T11:49:14Z

docs/source/examples/notebooks/parameterization/parameterization.ipynb

This notebook has an error, can you rerun it and check if the error is still there of it is legacy?

this was old and is fixed by re-running

brosaplanella · 2023-09-01T11:52:02Z

pybamm/models/full_battery_models/base_battery_model.py

+                "none",
+                "1",
+                "2",
+                "3",
+                "4",
+                "5",
+                "6",
+                "7",
+                "8",
+                "9",
+                "10",


Probably something for a future issue, but I wonder if we can have a way to not hard code when an option is an integer (similar issue occurs with the RC components in the ECM), as technically we could have as many as we wanted.

thanks, changed to check for positive integers

brosaplanella · 2023-09-01T12:56:34Z

pybamm/models/submodels/particle/base_particle.py

@@ -98,44 +98,63 @@ def _get_standard_concentration_variables(
            c_s_av = pybamm.r_average(c_s_xav)

        variables = {
-            f"{Domain} {phase_name}particle stoichiometry": c_s / c_scale,
-            f"{Domain} {phase_name}particle concentration": c_s / c_scale,
+            # Dimensional concentration


Why do we have stoichiometry and dimensionless concentration?

this is a hangover from when we had dimensional and dimensionless concentration. now they are equivalent. maybe we should remove dimensionless concentration now?

brosaplanella · 2023-09-01T12:59:46Z

pybamm/parameters/lithium_ion_parameters.py

+        # j0_j = (
+        #    j0_ref_j
+        #    * xj ** (wj * aj)
+        #    * (Xj - xj) ** (wj * (1 - aj))
+        #    * (c_e / c_e_ref) ** (1 - aj)
+        # )


Can this be deleted?

updated the comment to explain this is what the original formulation is in the paper

rtimms added 18 commits May 26, 2023 11:55

basic MSMR model with fast diffusion

e60ec43

run test simulation

094f2b6

particle problem in basic SP MSMR

6387858

update example

d1b20a7

merge develop

318f6d3

MSMR submodels for diffusion and ocp

e0e42d1

MSMR model options + example

c644be8

fix options test

e7f9c18

merge develop

8d92c18

esoh for msmr mostly working

dbf2018

start cleaning up esoh

3a7c39f

fix eSOH

ac821e8

improve esoh

0c61222

Use .diff to get dx/dU

f9a92be

esoh tests

86c4184

merge develop

c49e36f

fix diff for broadcasts

42c7438

start msmr notebook

4c0758d

rtimms marked this pull request as draft July 6, 2023 18:04

rtimms changed the title ~~MSMR model~~ [WIP] MSMR model Jul 6, 2023

rtimms requested a review from valentinsulzer July 6, 2023 18:07

valentinsulzer reviewed Jul 6, 2023

View reviewed changes

rtimms added 4 commits July 9, 2023 12:38

improve coverage

c94d842

update esoh

75835b5

clean up notebook

0f7c2ab

merge develop

51702ca

rtimms and others added 16 commits July 18, 2023 12:09

add individual reaction params

8c95a6d

fix setting by reaction index

085543f

add MSMR model class

899067f

add paramaters by reaction index

e9148ae

debugging current density

986d171

fix exchange current parameters

10bec45

fix PSD

e549a4e

merge develop

f7f8741

esoh coverage

78d2776

merge develop

925a04e

update notebook

fc0d71e

merge 'develop' into 'msmr'

c4953df

style: pre-commit fixes

d73a1eb

fix tests

32b73e5

merge conflicts

54a06e0

debug half cell

d8dace3

rtimms added 2 commits August 31, 2023 10:53

merge develop

9e513bf

relax option check

d1cd977

rtimms requested a review from brosaplanella August 31, 2023 10:54

brosaplanella approved these changes Sep 1, 2023

View reviewed changes

rtimms added 6 commits September 11, 2023 11:08

merge develop

d3d921d

ferran msmr comments

c030b88

update docs

f8a0fbc

fix example notebooks

c330ffa

merge develop

cb36b35

update bib in docs

77a62c1

rtimms merged commit e094094 into develop Sep 14, 2023
29 of 30 checks passed

kratman mentioned this pull request Sep 14, 2023

Replace deprecated pkg_resources #3335

Merged

5 tasks

kratman deleted the msmr branch March 19, 2024 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MSMR model #3116

MSMR model #3116

rtimms commented Jul 6, 2023

review-notebook-app bot commented Jul 6, 2023

codecov bot commented Jul 6, 2023 •

edited

Loading

valentinsulzer left a comment

valentinsulzer Jul 6, 2023

rtimms commented Jul 6, 2023

rtimms commented Jul 10, 2023 •

edited

Loading

rtimms commented Aug 11, 2023 •

edited

Loading

brosaplanella left a comment

brosaplanella Sep 1, 2023

brosaplanella Sep 1, 2023

rtimms Sep 11, 2023

brosaplanella Sep 1, 2023

rtimms Sep 11, 2023

brosaplanella Sep 1, 2023

rtimms Sep 11, 2023

brosaplanella Sep 1, 2023

rtimms Sep 11, 2023

MSMR model #3116

MSMR model #3116

Conversation

rtimms commented Jul 6, 2023

Description

Type of change

Key checklist:

Further checks:

review-notebook-app bot commented Jul 6, 2023

codecov bot commented Jul 6, 2023 • edited Loading

Codecov Report

valentinsulzer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtimms commented Jul 6, 2023

rtimms commented Jul 10, 2023 • edited Loading

rtimms commented Aug 11, 2023 • edited Loading

brosaplanella left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 6, 2023 •

edited

Loading

rtimms commented Jul 10, 2023 •

edited

Loading

rtimms commented Aug 11, 2023 •

edited

Loading