SEI on cracks by DrSOKane · Pull Request #2104 · pybamm-team/PyBaMM

DrSOKane · 2022-06-15T14:00:09Z

Description

Adds a new option "SEI on cracks". If set to "true", whichever SEI model is specified by the "SEI" option is run twice; once on the electrode particle surface, and then a second time on the additional surface created by the particle cracking created by the "particle mechanics": "swelling and cracking" option.

Breaking change: "Loss of lithium to SEI on cracks [mol]" is now a summary variable, so a "particle mechanics" submodel must be set, even if it's "none". The examples have been updated to account for this.

Fixes #1695 (in full), #1807 (in part), #2039 (in full)

Type of change

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: $ flake8
All tests pass: $ python run-tests.py --unit
The documentation builds: $ cd docs and then $ make clean; make html

You can run all three 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

DrSOKane · 2022-06-15T17:21:40Z

I thought I'd provide more context as to why this is still a draft.

If I do the following:

parameter_values = pybamm.ParameterValues("Ai2020")
spm = pybamm.lithium_ion.SPM({
"particle mechanics": "swelling and cracking",
"SEI": "solvent-diffusion limited",
"SEI on cracks": "true",
})

I get back a KeyError on line 231 of base_sei.py saying variables["Negative electrode roughness ratio"] is undefined. Presumably this is because this variable comes from a different submodel and was therefore not passed to the variables dictionary, but I have no idea how to fix this!

This reverts commit 5b321a7.

DrSOKane · 2022-06-27T11:56:37Z

Update: I have now got the SEI on cracks model to work for the Ai2020 parameter set. I'm now trying to add the degradation parameter set I used for my paper but the crack model fails immediately (t=0) for those parameters, even if there is no SEi on the cracks. This happens for either electrode, so the issue must be with one of the parameters common to both.

…SEIoncracks

valentinsulzer · 2022-07-15T18:54:12Z

I don't think this closes #1807 ?

…f-cell case

DrSOKane · 2022-07-19T13:14:49Z

I don't think this closes #1807 ?

#1807 is a wide-ranging issue that will take many PRs to close completely. SEI growth on cracks was mentioned, but not O'Kane et al. 2022.

codecov · 2022-07-19T13:38:21Z

Codecov Report

Merging #2104 (5276a9b) into develop (a5f8687) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##           develop    #2104    +/-   ##
=========================================
  Coverage    99.35%   99.36%            
=========================================
  Files          347      355     +8     
  Lines        19310    19451   +141     
=========================================
+ Hits         19186    19327   +141     
  Misses         124      124

Impacted Files	Coverage Δ
...ls/LGM50_ORegan2022/aluminium_heat_capacity_CRC.py	`100.00% <ø> (ø)`
...cells/LGM50_ORegan2022/copper_heat_capacity_CRC.py	`100.00% <ø> (ø)`
...GM50_ORegan2022/copper_thermal_conductivity_CRC.py	`100.00% <ø> (ø)`
...e_OKane2022/graphite_LGM50_diffusivity_Chen2020.py	`100.00% <ø> (ø)`
...0_electrolyte_exchange_current_density_Chen2020.py	`100.00% <ø> (ø)`
.../graphite_OKane2022/graphite_LGM50_ocp_Chen2020.py	`100.00% <ø> (ø)`
...graphite_ORegan2022/graphite_LGM50_ocp_Chen2020.py	`100.00% <ø> (ø)`
...lectrodes/nmc_ORegan2022/nmc_LGM50_ocp_Chen2020.py	`100.00% <ø> (ø)`
...amm/models/full_battery_models/lithium_ion/spme.py	`100.00% <ø> (ø)`
...raphite_OKane2022/graphite_cracking_rate_Ai2020.py	`100.00% <100.00%> (ø)`
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 468b39f...5276a9b. Read the comment docs.

…s acceptable)

DrSOKane · 2022-07-21T17:54:00Z

Sorry about the diff coverage. I'm getting an error when I try to test graphite_volume_change_Ai2020.py, so I commented that line out in the test and just dealt with everything else. Project coverage should be increased overall.

valentinsulzer

Looks great, some minor comments, including places where you are running up against the horrible mess of different parameter names for the same SEI parameter

valentinsulzer · 2022-07-22T18:10:25Z

+    T_ref = Parameter("Reference temperature [K]")
+    Eac_cr = Parameter(
+        "Negative electrode activation energy for cracking rate [J.mol-1]"
+    )
+    arrhenius = exp(Eac_cr / constants.R * (1 / T_dim - 1 / T_ref))


Please specify the activation energy and reference temperature here. The data is measured for a specific reference temperature and activation energy, so I don't think it makes sense to define them as separate parameters.

valentinsulzer · 2022-07-22T18:10:41Z

+    T_ref = Parameter("Reference temperature [K]")
+    Eac_cr = Parameter(
+        "Positive electrode activation energy for cracking rate [J.mol-1]"
+    )


valentinsulzer · 2022-07-22T18:20:51Z

+        # Do not set "sei on cracks" submodel for half-cells
+        if reaction_loc == "interface":
+            if self.options["SEI"] == "none":
+                self.submodels["sei"] = pybamm.sei.NoSEI(self.param, self.options)
+            elif self.options["SEI"] == "constant":
+                self.submodels["sei"] = pybamm.sei.ConstantSEI(self.param, self.options)
+            else:
+                self.submodels["sei"] = pybamm.sei.SEIGrowth(
+                    self.param, reaction_loc, self.options, cracks=False
+                )
+        # For full cells, "sei on cracks" submodel must be set, even if it is zero
        else:
-            self.submodels["sei"] = pybamm.sei.SEIGrowth(
-                self.param, reaction_loc, self.options
-            )
+            if self.options["SEI"] == "none":
+                self.submodels["sei"] = pybamm.sei.NoSEI(self.param, self.options)
+                self.submodels["sei on cracks"] = pybamm.sei.NoSEI(
+                    self.param, self.options, cracks=True
+                )
+            elif self.options["SEI"] == "constant":
+                self.submodels["sei"] = pybamm.sei.ConstantSEI(self.param, self.options)
+                self.submodels["sei on cracks"] = pybamm.sei.NoSEI(
+                    self.param, self.options, cracks=True
+                )
+            else:
+                self.submodels["sei"] = pybamm.sei.SEIGrowth(
+                    self.param, reaction_loc, self.options, cracks=False
+                )
+                if self.options["SEI on cracks"] == "true":
+                    self.submodels["sei on cracks"] = pybamm.sei.SEIGrowth(
+                        self.param, reaction_loc, self.options, cracks=True
+                    )
+                else:
+                    self.submodels["sei on cracks"] = pybamm.sei.NoSEI(
+                        self.param, self.options, cracks=True
+                    )


I think this does the same thing in fewer lines

if self.options["SEI"] == "none": self.submodels["sei"] = pybamm.sei.NoSEI(self.param, self.options) elif self.options["SEI"] == "constant": self.submodels["sei"] = pybamm.sei.ConstantSEI(self.param, self.options) else: self.submodels["sei"] = pybamm.sei.SEIGrowth( self.param, reaction_loc, self.options, cracks=False ) # Do not set "sei on cracks" submodel for half-cells # For full cells, "sei on cracks" submodel must be set, even if it is zero if reaction_loc != "interface": if self.options["SEI"] in ["none", "constant"] or self.options["SEI on cracks"] == "false": self.submodels["sei on cracks"] = pybamm.sei.NoSEI( self.param, self.options, cracks=True ) else: self.submodels["sei on cracks"] = pybamm.sei.SEIGrowth( self.param, reaction_loc, self.options, cracks=True )

valentinsulzer · 2022-07-22T18:26:28Z

        if self.options["SEI"] == "ec reaction limited":
-            self.initial_conditions = {L_outer: L_inner_0 + L_outer_0}
+            if self.reaction == "SEI on cracks":
+                self.initial_conditions = {L_outer: (L_inner_0 + L_outer_0) / 10000}


why hard-coded to 10000?

That is what was done in the paper. We had to choose an initial condition that was as close to zero to get the physics right, but didn't cause a division by zero error.

valentinsulzer · 2022-07-22T18:29:43Z

+        # All SEI growth mechanisms assumed to have Arrhenius dependence
+        Arrhenius = pybamm.exp(param.E_over_RT_sei * (1 - prefactor))

-        j_inner = alpha * j_sei
-        j_outer = (1 - alpha) * j_sei
+        j_inner = alpha * Arrhenius * j_sei
+        j_outer = (1 - alpha) * Arrhenius * j_sei


I understand why you've done it this way, but we should reformat the SEI parameters so that the parameter is j_sei(c, T) to be consistent with intercalation kinetics and plating. The SEI parameters are a mess anyway.

This is a good idea but probably beyond the scope of this PR, which is to allow others to reproduce the results of the O'Kane 2022 paper.

brosaplanella · 2022-07-25T12:05:14Z

+
+def graphite_cracking_rate_Ai2020(T_dim):
+    """
+    graphite particle cracking rate as a function of temperature [1, 2].


Suggested change

graphite particle cracking rate as a function of temperature [1, 2].

Graphite particle cracking rate as a function of temperature [1, 2].

brosaplanella · 2022-07-25T12:38:35Z

+                    domain="negative electrode",
+                    auxiliary_domains={"secondary": "current collector"},
+                )
+        else:


Does this case include standard SEI only, or a bunch of different cases? If it is standard SEI, would it be better to do an elif and then keep the else to throw an error if the argument is weird?

What error should I throw?

brosaplanella · 2022-07-25T12:44:15Z

@@ -29,6 +29,24 @@ Negative electrode specific heat capacity [J.kg-1.K-1],700,default,
 Negative electrode thermal conductivity [W.m-1.K-1],1.7,default,


Are all these parameters presented in O'Kane 2022? If so, should we rename this set to O'Kane 2022 rather than Chen 2020 plating? We can still keep the ref to Chen 2020 in pybamm.citations() but I think the new name would make it clearer to the reader which is the main reference.

I've gone further and deleted the entire Chen2020_plating chemistry. It was only there in the first place because none of the existing chemistries worked with the plating submodel. Now that OKane2022 exists, it's not needed anymore. OKane2020_Li_plating is still there as that is a published set of plating parameters, but none of the pre-defined chemistries use it.

brosaplanella · 2022-07-25T12:45:54Z

+
+class NoMechanics(BaseMechanics):
+    """
+    Class for swelling only (no cracking)


Is this right?

valentinsulzer

Looks good to me! Will let @brosaplanella double-check

valentinsulzer · 2022-07-26T17:36:59Z

+        where m_cr is another Paris' law constant
+    """
+    k_cr = 3.9e-20
+    Eac_cr = Parameter(


this should be hard-coded too

Fixed (currently 0, sorry to disappoint!)

valentinsulzer · 2022-07-26T17:37:08Z

+        where m_cr is another Paris' law constant
+    """
+    k_cr = 3.9e-20
+    Eac_cr = Parameter(


should be hard-coded

Fixed (currently 0, sorry to disappoint!)

valentinsulzer · 2022-07-26T18:17:18Z

+Negative electrode Paris' law constant b,1.12,,
+Negative electrode Paris' law constant m,2.2,,
+Negative electrode cracking rate,[function]graphite_cracking_rate_Ai2020,Ai2020,
+Negative electrode activation energy for cracking rate [J.mol-1],0,,


these can be removed

valentinsulzer · 2022-07-26T18:17:34Z

+Positive electrode Paris' law constant b,1.12,,
+Positive electrode Paris' law constant m,2.2,,
+Positive electrode cracking rate,[function]cracking_rate_Ai2020,Ai2020,
+Positive electrode activation energy for cracking rate [J.mol-1],0,,


can be removed

brosaplanella

Looks good to me!

valentinsulzer · 2022-07-27T15:11:51Z

You just lost to Ferran in the merge race 😢 just fix the changelog conflict and should be good to go

DrSOKane added 6 commits April 27, 2022 11:24

Partially completed first draft of SEI on cracks

b27ae28

Second attempt at adding SEI on cracks

23a0a6f

Fixed some bugs in crack SEI model

d17473a

Ensure loss of lithium to SEI on cracks is always defined

e105d40

Replaced most references to 'SEI' and 'SEI on cracks' with self.reaction

e4d93d8

Changed L_n to n.L

e36602b

DrSOKane added 3 commits June 20, 2022 16:30

Added NoMechanics subclass

ecfcdc4

flake8

fea7d4c

Added if statement to set roughness to 1 if not in variables

5b321a7

valentinsulzer reviewed Jun 21, 2022

View reviewed changes

Comment thread pybamm/models/submodels/interface/sei/base_sei.py Outdated

DrSOKane added 5 commits June 22, 2022 14:52

Revert "Added if statement to set roughness to 1 if not in variables"

6bfddd1

This reverts commit 5b321a7.

Merge branch 'develop' into SEIoncracks

3fb424a

Added cracking parameters to graphite_Chen2020_plating

775626f

Added get_coupled_variables to NoSEI class

0ad8afe

Cracking model works but OKane2022 parameters do not

a21484d

DrSOKane added 6 commits June 29, 2022 13:51

Added Arrhenius temperature dependence to all SEI growth models

7ec5fc8

Merge branch 'develop' of https://github.com/pybamm-team/PyBaMM into …

274439d

…SEIoncracks

Added zero temperature dependence for all other SEI parameter sets

e106c1d

Completed example notebook

82d5800

Updated with latest changes from upstream

17725e5

Updated CHANGELOG

3dc6375

DrSOKane added 4 commits July 18, 2022 16:52

flake8

c78229a

Fixed get_coupled_variables in no_sei.py

6e7fbcf

Changed base_lithium_ion_model to avoid running SEI on cracks for hal…

0a93993

…f-cell case

flake8

2ddb0c2

DrSOKane added 4 commits July 20, 2022 13:15

Fixed bug in equation for interstitial-diffusion limited SEI

0080825

Fixed examples, added SEI on cracks porosity change, added tests

c628f95

flake8

f23b3ba

Breaking change: particle mechanics now a compulsory submodel (none i…

5f0258e

…s acceptable)

DrSOKane marked this pull request as ready for review July 21, 2022 15:10

coverage

7ee18d8

DrSOKane added 4 commits July 21, 2022 18:59

Fixed tests

1d6319e

Merged latest changes from develop to fix conflict

ffdbb48

Fixed test_OKane2022

f6b979f

flake8

5ae0a6c

valentinsulzer requested changes Jul 22, 2022

View reviewed changes

brosaplanella reviewed Jul 25, 2022

View reviewed changes

valentinsulzer mentioned this pull request Jul 26, 2022

add temperature dependense in SEI model #2080

Closed

8 tasks

DrSOKane added 4 commits July 26, 2022 13:37

Completed changes requested in review

63a9c79

changelog

21f0374

Modified tests to account for Chen2020_plating being deleted

4153103

Modified more tests to account for Chen2020_plating being deleted

9ad8b75

valentinsulzer approved these changes Jul 26, 2022

View reviewed changes

Hard coded cracking activation energy (currently 0)

ebeffb1

valentinsulzer reviewed Jul 26, 2022

View reviewed changes

brosaplanella approved these changes Jul 27, 2022

View reviewed changes

DrSOKane added 3 commits July 27, 2022 12:23

Removed cracking activation energy from parameter csv files

51d1c4f

Deleted the correct parameter this time

851b44c

Third time lucky?

2b77c81

Fixed conflict in changelog

5276a9b

valentinsulzer merged commit 58194b0 into pybamm-team:develop Jul 27, 2022

	graphite particle cracking rate as a function of temperature [1, 2].
	Graphite particle cracking rate as a function of temperature [1, 2].

		@@ -29,6 +29,24 @@ Negative electrode specific heat capacity [J.kg-1.K-1],700,default,
		Negative electrode thermal conductivity [W.m-1.K-1],1.7,default,

Uh oh!

Conversation

DrSOKane commented Jun 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Key checklist:

Further checks:

Uh oh!

DrSOKane commented Jun 15, 2022

Uh oh!

Uh oh!

DrSOKane commented Jun 27, 2022

Uh oh!

valentinsulzer commented Jul 15, 2022

Uh oh!

DrSOKane commented Jul 19, 2022

Uh oh!

codecov Bot commented Jul 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DrSOKane commented Jul 21, 2022

Uh oh!

valentinsulzer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valentinsulzer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrSOKane commented Jun 15, 2022 •

edited

Loading

codecov Bot commented Jul 19, 2022 •

edited

Loading