#1781 merge test reutilization

pybamm-team · Nov 17, 2021 · 5feea0c · 5feea0c
2 parents 2de2a47 + e52a1bb
commit 5feea0c
Show file tree

Hide file tree

Showing 42 changed files with 738 additions and 2,823 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Features
 
+-   Reformatted SEI growth models into a single submodel with conditionals ([#1808](https://github.com/pybamm-team/PyBaMM/pull/1808))
 -   Stress-induced diffusion is now a separate model option instead of being automatically included when using the particle mechanics submodels ([#1797](https://github.com/pybamm-team/PyBaMM/pull/1797))
 -   `Experiment`s with drive cycles can be solved ([#1793](https://github.com/pybamm-team/PyBaMM/pull/1793))
 -   Added surface area to volume ratio as a factor to the SEI equations ([#1790](https://github.com/pybamm-team/PyBaMM/pull/1790))

diff --git a/docs/source/models/submodels/interface/sei.rst b/docs/source/models/submodels/interface/sei.rst
@@ -7,20 +7,8 @@ SEI models
 .. autoclass:: pybamm.sei.ConstantSEI
     :members:
 
-.. autoclass:: pybamm.sei.ElectronMigrationLimited
-    :members:
-
-.. autoclass:: pybamm.sei.InterstitialDiffusionLimited
+.. autoclass:: pybamm.sei.SEIGrowth
     :members:
 
 .. autoclass:: pybamm.sei.NoSEI
     :members:
-
-.. autoclass:: pybamm.sei.ReactionLimited
-    :members:
-
-.. autoclass:: pybamm.sei.SolventDiffusionLimited
-    :members:
-
-.. autoclass:: pybamm.sei.EcReactionLimited
-    :members:
diff --git a/pybamm/expression_tree/unary_operators.py b/pybamm/expression_tree/unary_operators.py
@@ -8,7 +8,6 @@
 from scipy.sparse import csr_matrix, issparse
 from sympy.vector.operators import Divergence as sympy_Divergence
 from sympy.vector.operators import Gradient as sympy_Gradient
-
 import pybamm
 
 
@@ -152,17 +151,21 @@ def __init__(self, child):
     def diff(self, variable):
         """See :meth:`pybamm.Symbol.diff()`."""
         child = self.child.new_copy()
-        return Sign(child) * child.diff(variable)
+        return sign(child) * child.diff(variable)
 
     def _unary_jac(self, child_jac):
         """See :meth:`pybamm.UnaryOperator._unary_jac()`."""
         child = self.child.new_copy()
-        return Sign(child) * child_jac
+        return sign(child) * child_jac
 
     def _unary_evaluate(self, child):
         """See :meth:`UnaryOperator._unary_evaluate()`."""
         return np.abs(child)
 
+    def _unary_new_copy(self, child):
+        """See :meth:`UnaryOperator._unary_new_copy()`."""
+        return abs(child)
+
 
 class Sign(UnaryOperator):
     """
@@ -188,7 +191,16 @@ def _unary_evaluate(self, child):
         if issparse(child):
             return csr_matrix.sign(child)
         else:
-            return np.sign(child)
+            with np.errstate(invalid="ignore"):
+                return np.sign(child)
+
+    def _unary_new_copy(self, child):
+        """See :meth:`UnaryOperator._unary_new_copy()`."""
+        return sign(child)
+
+    def _sympy_operator(self, child):
+        """Override :meth:`pybamm.UnaryOperator._sympy_operator`"""
+        return sympy.functions.elementary.complexes.sign(child)
 
 
 class Floor(UnaryOperator):

diff --git a/pybamm/models/full_battery_models/base_battery_model.py b/pybamm/models/full_battery_models/base_battery_model.py
@@ -87,15 +87,9 @@ class BatteryModelOptions(pybamm.FuzzyDict):
 
                 - "none": :class:`pybamm.sei.NoSEI` (no SEI growth)
                 - "constant": :class:`pybamm.sei.Constant` (constant SEI thickness)
-                - "reaction limited": :class:`pybamm.sei.ReactionLimited`
-                - "solvent-diffusion limited":\
-                    :class:`pybamm.sei.SolventDiffusionLimited`
-                - "electron-migration limited": \
-                    :class:`pybamm.sei.ElectronMigrationLimited`
-                - "interstitial-diffusion limited": \
-                    :class:`pybamm.sei.InterstitialDiffusionLimited`
-                - "ec reaction limited": \
-                    :class:`pybamm.sei.EcReactionLimited`
+                - "reaction limited", "solvent-diffusion limited",\
+                    "electron-migration limited", "interstitial-diffusion limited", \
+                    or "ec reaction limited": :class:`pybamm.sei.SEIGrowth`
             * "SEI film resistance" : str
                 Set the submodel for additional term in the overpotential due to SEI.
                 The default value is "none" if the "SEI" option is "none", and
@@ -338,6 +332,13 @@ def __init__(self, extra_options):
                 raise pybamm.OptionError(
                     "cannot have transverse convection in 0D model"
                 )
+
+        if options["thermal"] == "x-full" and options["dimensionality"] != 0:
+            n = options["dimensionality"]
+            raise pybamm.OptionError(
+                f"X-full thermal submodels do not yet support {n}D current collectors"
+            )
+
         if isinstance(options["stress-induced diffusion"], str):
             if (
                 options["stress-induced diffusion"] == "true"
@@ -836,11 +837,9 @@ def set_thermal_submodel(self):
                 thermal_submodel = pybamm.thermal.pouch_cell.CurrentCollector2D(
                     self.param
                 )
-
-        elif (
-            self.options["thermal"] == "x-full" and self.options["dimensionality"] == 0
-        ):
-            thermal_submodel = pybamm.thermal.OneDimensionalX(self.param)
+        elif self.options["thermal"] == "x-full":
+            if self.options["dimensionality"] == 0:
+                thermal_submodel = pybamm.thermal.OneDimensionalX(self.param)
 
         self.submodels["thermal"] = thermal_submodel
 

diff --git a/pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py b/pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py
@@ -201,24 +201,8 @@ def set_sei_submodel(self):
             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)
-        elif self.options["SEI"] == "reaction limited":
-            self.submodels["sei"] = pybamm.sei.ReactionLimited(
-                self.param, reaction_loc, self.options
-            )
-        elif self.options["SEI"] == "solvent-diffusion limited":
-            self.submodels["sei"] = pybamm.sei.SolventDiffusionLimited(
-                self.param, reaction_loc, self.options
-            )
-        elif self.options["SEI"] == "electron-migration limited":
-            self.submodels["sei"] = pybamm.sei.ElectronMigrationLimited(
-                self.param, reaction_loc, self.options
-            )
-        elif self.options["SEI"] == "interstitial-diffusion limited":
-            self.submodels["sei"] = pybamm.sei.InterstitialDiffusionLimited(
-                self.param, reaction_loc, self.options
-            )
-        elif self.options["SEI"] == "ec reaction limited":
-            self.submodels["sei"] = pybamm.sei.EcReactionLimited(
+        else:
+            self.submodels["sei"] = pybamm.sei.SEIGrowth(
                 self.param, reaction_loc, self.options
             )
 
@@ -227,14 +211,10 @@ def set_lithium_plating_submodel(self):
             self.submodels["lithium plating"] = pybamm.lithium_plating.NoPlating(
                 self.param, self.options
             )
-        elif self.options["lithium plating"] == "reversible":
-            self.submodels[
-                "lithium plating"
-            ] = pybamm.lithium_plating.ReversiblePlating(self.param, self.x_average)
-        elif self.options["lithium plating"] == "irreversible":
-            self.submodels[
-                "lithium plating"
-            ] = pybamm.lithium_plating.IrreversiblePlating(self.param, self.x_average)
+        else:
+            self.submodels["lithium plating"] = pybamm.lithium_plating.Plating(
+                self.param, self.x_average, self.options
+            )
 
     def set_other_reaction_submodels_to_zero(self):
         self.submodels["negative oxygen interface"] = pybamm.interface.NoReaction(

diff --git a/pybamm/models/full_battery_models/lithium_ion/newman_tobias.py b/pybamm/models/full_battery_models/lithium_ion/newman_tobias.py
@@ -56,31 +56,31 @@ def __init__(self, options=None, name="Newman-Tobias model", build=True):
         pybamm.citations.register("Chu2020")
 
     def set_particle_submodel(self):
-
-        if self.options["particle"] == "Fickian diffusion":
-            submod_n = pybamm.particle.no_distribution.XAveragedFickianDiffusion(
-                self.param, "Negative", self.options
-            )
-            self.submodels["negative particle"] = submod_n
-            submod_p = pybamm.particle.no_distribution.XAveragedFickianDiffusion(
-                self.param, "Positive", self.options
-            )
-            self.submodels["positive particle"] = submod_p
-        elif self.options["particle"] in [
-            "uniform profile",
-            "quadratic profile",
-            "quartic profile",
+        if isinstance(self.options["particle"], str):
+            particle_left = self.options["particle"]
+            particle_right = self.options["particle"]
+        else:
+            particle_left, particle_right = self.options["particle"]
+        for particle_side, domain in [
+            [particle_left, "Negative"],
+            [particle_right, "Positive"],
         ]:
-            self.submodels[
-                "negative particle"
-            ] = pybamm.particle.no_distribution.XAveragedPolynomialProfile(
-                self.param, "Negative", self.options["particle"], self.options
-            )
-            self.submodels[
-                "positive particle"
-            ] = pybamm.particle.no_distribution.XAveragedPolynomialProfile(
-                self.param, "Positive", self.options["particle"], self.options
-            )
+            if particle_side == "Fickian diffusion":
+                self.submodels[
+                    domain.lower() + " particle"
+                ] = pybamm.particle.no_distribution.XAveragedFickianDiffusion(
+                    self.param, domain, self.options
+                )
+            elif particle_side in [
+                "uniform profile",
+                "quadratic profile",
+                "quartic profile",
+            ]:
+                self.submodels[
+                    domain.lower() + " particle"
+                ] = pybamm.particle.no_distribution.XAveragedPolynomialProfile(
+                    self.param, domain, particle_side, self.options
+                )
 
     def set_electrolyte_submodel(self):
 

diff --git a/pybamm/models/submodels/interface/lithium_plating/__init__.py b/pybamm/models/submodels/interface/lithium_plating/__init__.py
@@ -1,4 +1,3 @@
 from .base_plating import BasePlating
 from .no_plating import NoPlating
-from .reversible_plating import ReversiblePlating
-from .irreversible_plating import IrreversiblePlating
+from .plating import Plating
diff --git a/pybamm/models/submodels/interface/lithium_plating/base_plating.py b/pybamm/models/submodels/interface/lithium_plating/base_plating.py
@@ -20,6 +20,7 @@ class BasePlating(BaseInterface):
            Origin of the Differential Voltage Minimum Associated with Li Plating in
            Lithium-Ion Batteries". Journal of The Electrochemical Society,
            167:090540, 2019
+
     **Extends:** :class:`pybamm.interface.BaseInterface`
     """
 

diff --git a/...e/lithium_plating/irreversible_plating.py → ...dels/interface/lithium_plating/plating.py b/...e/lithium_plating/irreversible_plating.py → ...dels/interface/lithium_plating/plating.py
@@ -1,18 +1,21 @@
 #
-# Class for irreversible lithium plating
+# Class for lithium plating
 #
 import pybamm
 from .base_plating import BasePlating
 
 
-class IrreversiblePlating(BasePlating):
-    """Base class for irreversible lithium plating.
+class Plating(BasePlating):
+    """Class for lithium plating.
+
     Parameters
     ----------
     param : parameter class
         The parameters to use for this submodel
     x_average : bool
         Whether to use x-averaged variables (SPM, SPMe, etc) or full variables (DFN)
+    options : dict, optional
+        A dictionary of options to be passed to the model.
 
     References
     ----------
@@ -24,8 +27,8 @@ class IrreversiblePlating(BasePlating):
     **Extends:** :class:`pybamm.lithium_plating.BasePlating`
     """
 
-    def __init__(self, param, x_average):
-        super().__init__(param)
+    def __init__(self, param, x_average, options):
+        super().__init__(param, options)
         self.x_average = x_average
         pybamm.citations.register("OKane2020")
 
@@ -54,14 +57,22 @@ def get_coupled_variables(self, variables):
         T = variables["Negative electrode temperature"]
         eta_sei = variables["SEI film overpotential"]
         c_plated_Li = variables["Lithium plating concentration"]
+        j0_stripping = param.j0_stripping(c_e_n, c_plated_Li, T)
         j0_plating = param.j0_plating(c_e_n, c_plated_Li, T)
         phi_ref = param.U_n_ref / param.potential_scale
 
         eta_stripping = delta_phi + phi_ref + eta_sei
         eta_plating = -eta_stripping
         prefactor = 1 / (2 * (1 + self.param.Theta * T))
-        # j_stripping is always negative, because there is no stripping, only plating
-        j_stripping = -j0_plating * pybamm.exp(prefactor * eta_plating)
+
+        if self.options["lithium plating"] == "reversible":
+            j_stripping = j0_stripping * pybamm.exp(
+                prefactor * eta_stripping
+            ) - j0_plating * pybamm.exp(prefactor * eta_plating)
+        elif self.options["lithium plating"] == "irreversible":
+            # j_stripping is always negative, because there is no stripping, only
+            # plating
+            j_stripping = -j0_plating * pybamm.exp(prefactor * eta_plating)
 
         variables.update(self._get_standard_overpotential_variables(eta_stripping))
         variables.update(self._get_standard_reaction_variables(j_stripping))