Merge pull request #1707 from pybamm-team/experiment-bugs

fix experiment bugs

CHANGELOG.md

@@ -11,6 +11,9 @@
 
 -   Fixed `sympy` operators for `Arctan` and `Exponential` ([#1786](https://github.com/pybamm-team/PyBaMM/pull/1786))
 -   Fixed finite volume discretization in spherical polar coordinates ([#1782](https://github.com/pybamm-team/PyBaMM/pull/1782))
+-   Fixed bug when using `Experiment` with a pouch cell model ([#1707](https://github.com/pybamm-team/PyBaMM/pull/1707))
+-   Fixed bug when using `Experiment` with a plating model ([#1707](https://github.com/pybamm-team/PyBaMM/pull/1707))
+-   Fixed hack for potentials in the SPMe model ([#1707](https://github.com/pybamm-team/PyBaMM/pull/1707))
 
 ## Breaking changes
 

docs/source/models/submodels/electrolyte_conductivity/surface_form/explicit_surface_form_conductivity.rst

@@ -0,0 +1,5 @@
+Explicit Model
+==============
+
+.. autoclass:: pybamm.electrolyte_conductivity.surface_potential_form.Explicit
+    :members:

docs/source/models/submodels/electrolyte_conductivity/surface_form/index.rst

@@ -5,3 +5,4 @@ Surface Form
 
   full_surface_form_conductivity
   leading_surface_form_conductivity
+  explicit_surface_form_conductivity

examples/scripts/custom_model.py

@@ -59,6 +59,16 @@
 model.submodels[
     "electrolyte conductivity"
 ] = pybamm.electrolyte_conductivity.LeadingOrder(model.param)
+model.submodels[
+    "negative surface potential difference"
+] = pybamm.electrolyte_conductivity.surface_potential_form.Explicit(
+    model.param, "Negative"
+)
+model.submodels[
+    "positive surface potential difference"
+] = pybamm.electrolyte_conductivity.surface_potential_form.Explicit(
+    model.param, "Positive"
+)
 model.submodels["sei"] = pybamm.sei.NoSEI(model.param)
 model.submodels["lithium plating"] = pybamm.lithium_plating.NoPlating(model.param)
 

pybamm/expression_tree/operations/replace_symbols.py

@@ -15,15 +15,23 @@ class SymbolReplacer(object):
         Map of which symbols should be replaced by which.
     processed_symbols: dict {variable ids -> :class:`pybamm.Symbol`}, optional
         cached replaced symbols
+    process_initial_conditions: bool, optional
+        Whether to process initial conditions, default is True
     """
 
-    def __init__(self, symbol_replacement_map, processed_symbols=None):
+    def __init__(
+        self,
+        symbol_replacement_map,
+        processed_symbols=None,
+        process_initial_conditions=True,
+    ):
         self._symbol_replacement_map = symbol_replacement_map
         self._symbol_replacement_map_ids = {
             symbol_in.id: symbol_out
             for symbol_in, symbol_out in symbol_replacement_map.items()
         }
         self._processed_symbols = processed_symbols or {}
+        self.process_initial_conditions = process_initial_conditions
 
     def process_model(self, unprocessed_model, inplace=True):
         """Replace all instances of a symbol in a model.
@@ -68,9 +76,12 @@ def process_model(self, unprocessed_model, inplace=True):
             pybamm.logger.verbose(
                 "Replacing symbols in {!r} (initial conditions)".format(variable)
             )
-            new_initial_conditions[self.process_symbol(variable)] = self.process_symbol(
-                equation
-            )
+            if self.process_initial_conditions:
+                new_initial_conditions[
+                    self.process_symbol(variable)
+                ] = self.process_symbol(equation)
+            else:
+                new_initial_conditions[self.process_symbol(variable)] = equation
         model.initial_conditions = new_initial_conditions
 
         model.boundary_conditions = self.process_boundary_conditions(unprocessed_model)

pybamm/models/base_model.py

@@ -170,6 +170,20 @@ def variables(self):
 
     @variables.setter
     def variables(self, variables):
+        for name, var in variables.items():
+            if (
+                isinstance(var, pybamm.Variable)
+                and var.name != name
+                # Exception if the variable is also there under its own name
+                and not (var.name in variables and variables[var.name].id == var.id)
+                # Exception for the key "Leading-order"
+                and "leading-order" not in var.name.lower()
+                and "leading-order" not in name.lower()
+            ):
+                raise ValueError(
+                    f"Variable with name '{var.name}' is in variables dictionary with "
+                    f"name '{name}'. Names must match."
+                )
         self._variables = pybamm.FuzzyDict(variables)
 
     def variable_names(self):

pybamm/models/full_battery_models/base_battery_model.py

@@ -683,7 +683,7 @@ def build_coupled_variables(self):
                                 )
                             )
         # Convert variables back into FuzzyDict
-        self._variables = pybamm.FuzzyDict(self._variables)
+        self.variables = pybamm.FuzzyDict(self._variables)
 
     def build_model_equations(self):
         # Set model equations
@@ -749,17 +749,6 @@ def build_model(self):
         self.set_degradation_variables()
         self.set_summary_variables()
 
-        # Massive hack for consistent delta_phi = phi_s - phi_e with SPMe
-        # This needs to be corrected
-        if isinstance(self, pybamm.lithium_ion.SPMe) and not self.half_cell:
-            for domain in ["Negative", "Positive"]:
-                phi_s = self.variables[domain + " electrode potential"]
-                phi_e = self.variables[domain + " electrolyte potential"]
-                delta_phi = phi_s - phi_e
-                s = self.submodels[domain.lower() + " interface"]
-                var = s._get_standard_surface_potential_difference_variables(delta_phi)
-                self.variables.update(var)
-
         self._built = True
         pybamm.logger.info("Finish building {}".format(self.name))
 

pybamm/models/full_battery_models/lead_acid/full.py

@@ -110,16 +110,16 @@ def set_electrolyte_submodel(self):
             self.submodels[
                 "electrolyte conductivity"
             ] = pybamm.electrolyte_conductivity.Full(self.param)
+            surf_model = surf_form.Explicit
         elif self.options["surface form"] == "differential":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    domain.lower() + " electrolyte conductivity"
-                ] = surf_form.FullDifferential(self.param, domain)
+            surf_model = surf_form.FullDifferential
         elif self.options["surface form"] == "algebraic":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    domain.lower() + " electrolyte conductivity"
-                ] = surf_form.FullAlgebraic(self.param, domain)
+            surf_model = surf_form.FullAlgebraic
+
+        for domain in ["Negative", "Separator", "Positive"]:
+            self.submodels[
+                domain.lower() + " surface potential difference"
+            ] = surf_model(self.param, domain)
 
     def set_side_reaction_submodels(self):
         if self.options["hydrolysis"] == "true":

pybamm/models/full_battery_models/lead_acid/loqs.py

@@ -196,18 +196,16 @@ def set_electrolyte_submodel(self):
             self.submodels[
                 "leading-order electrolyte conductivity"
             ] = pybamm.electrolyte_conductivity.LeadingOrder(self.param)
-
+            surf_model = surf_form.Explicit
         elif self.options["surface form"] == "differential":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    "leading-order " + domain.lower() + " electrolyte conductivity"
-                ] = surf_form.LeadingOrderDifferential(self.param, domain)
-
+            surf_model = surf_form.LeadingOrderDifferential
         elif self.options["surface form"] == "algebraic":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    "leading-order " + domain.lower() + " electrolyte conductivity"
-                ] = surf_form.LeadingOrderAlgebraic(self.param, domain)
+            surf_model = surf_form.LeadingOrderAlgebraic
+
+        for domain in ["Negative", "Positive"]:
+            self.submodels[
+                domain.lower() + " surface potential difference"
+            ] = surf_model(self.param, domain)
 
         self.submodels[
             "electrolyte diffusion"

pybamm/models/full_battery_models/lithium_ion/dfn.py

@@ -138,6 +138,25 @@ def set_solid_submodel(self):
         self.submodels["negative electrode potential"] = submod_n
         self.submodels["positive electrode potential"] = submod_p
 
+        # Set the counter-electrode model for the half-cell model
+        # The negative electrode model will be ignored
+        if self.half_cell:
+            if self.options["SEI"] in ["none", "constant"]:
+                self.submodels[
+                    "counter electrode surface potential difference"
+                ] = pybamm.electrolyte_conductivity.surface_potential_form.Explicit(
+                    self.param, "Negative", self.options
+                )
+                self.submodels[
+                    "counter electrode potential"
+                ] = pybamm.electrode.ohm.LithiumMetalExplicit(self.param, self.options)
+            else:
+                self.submodels[
+                    "counter electrode potential"
+                ] = pybamm.electrode.ohm.LithiumMetalSurfaceForm(
+                    self.param, self.options
+                )
+
     def set_electrolyte_submodel(self):
 
         surf_form = pybamm.electrolyte_conductivity.surface_potential_form
@@ -157,13 +176,13 @@ def set_electrolyte_submodel(self):
             self.submodels[
                 "electrolyte conductivity"
             ] = pybamm.electrolyte_conductivity.Full(self.param, self.options)
+            surf_model = surf_form.Explicit
         elif self.options["surface form"] == "differential":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    domain.lower() + " electrolyte conductivity"
-                ] = surf_form.FullDifferential(self.param, domain)
+            surf_model = surf_form.FullDifferential
         elif self.options["surface form"] == "algebraic":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    domain.lower() + " electrolyte conductivity"
-                ] = surf_form.FullAlgebraic(self.param, domain)
+            surf_model = surf_form.FullAlgebraic
+
+        for domain in ["Negative", "Separator", "Positive"]:
+            self.submodels[
+                domain.lower() + " surface potential difference"
+            ] = surf_model(self.param, domain, self.options)

pybamm/models/full_battery_models/lithium_ion/newman_tobias.py

@@ -101,13 +101,13 @@ def set_electrolyte_submodel(self):
             self.submodels[
                 "electrolyte conductivity"
             ] = pybamm.electrolyte_conductivity.Full(self.param)
+            surf_model = surf_form.Explicit
         elif self.options["surface form"] == "differential":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    domain.lower() + " electrolyte conductivity"
-                ] = surf_form.FullDifferential(self.param, domain)
+            surf_model = surf_form.FullDifferential
         elif self.options["surface form"] == "algebraic":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    domain.lower() + " electrolyte conductivity"
-                ] = surf_form.FullAlgebraic(self.param, domain)
+            surf_model = surf_form.FullAlgebraic
+
+        for domain in ["Negative", "Separator", "Positive"]:
+            self.submodels[
+                domain.lower() + " surface potential difference"
+            ] = surf_model(self.param, domain)

pybamm/models/full_battery_models/lithium_ion/spm.py

@@ -156,18 +156,16 @@ def set_electrolyte_submodel(self):
             ] = pybamm.electrolyte_conductivity.LeadingOrder(
                 self.param, options=self.options
             )
-
+            surf_model = surf_form.Explicit
         elif self.options["surface form"] == "differential":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    "leading-order " + domain.lower() + " electrolyte conductivity"
-                ] = surf_form.LeadingOrderDifferential(self.param, domain)
-
+            surf_model = surf_form.LeadingOrderDifferential
         elif self.options["surface form"] == "algebraic":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    "leading-order " + domain.lower() + " electrolyte conductivity"
-                ] = surf_form.LeadingOrderAlgebraic(self.param, domain)
+            surf_model = surf_form.LeadingOrderAlgebraic
+
+        for domain in ["Negative", "Positive"]:
+            self.submodels[
+                domain.lower() + " surface potential difference"
+            ] = surf_model(self.param, domain)
 
         self.submodels[
             "electrolyte diffusion"

pybamm/models/full_battery_models/lithium_ion/spme.py

@@ -117,16 +117,16 @@ def set_electrolyte_submodel(self):
                 ] = pybamm.electrolyte_conductivity.Integrated(
                     self.param, options=self.options
                 )
+            surf_model = surf_form.Explicit
         elif self.options["surface form"] == "differential":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    domain.lower() + " electrolyte conductivity"
-                ] = surf_form.CompositeDifferential(self.param, domain)
+            surf_model = surf_form.CompositeDifferential
         elif self.options["surface form"] == "algebraic":
-            for domain in ["Negative", "Separator", "Positive"]:
-                self.submodels[
-                    domain.lower() + " electrolyte conductivity"
-                ] = surf_form.CompositeAlgebraic(self.param, domain)
+            surf_model = surf_form.CompositeAlgebraic
+
+        for domain in ["Negative", "Positive"]:
+            self.submodels[
+                domain.lower() + " surface potential difference"
+            ] = surf_model(self.param, domain)
 
         self.submodels["electrolyte diffusion"] = pybamm.electrolyte_diffusion.Full(
             self.param, self.options

pybamm/models/submodels/convection/transverse/full_convection.py

@@ -22,7 +22,9 @@ def __init__(self, param):
 
     def get_fundamental_variables(self):
 
-        p_s = pybamm.Variable("Separator pressure", domain="current collector")
+        p_s = pybamm.Variable(
+            "X-averaged separator pressure", domain="current collector"
+        )
         variables = self._get_standard_separator_pressure_variables(p_s)
 
         # TODO: put in permeability and viscosity

pybamm/models/submodels/current_collector/composite_potential_pair.py

@@ -39,6 +39,13 @@ def get_fundamental_variables(self):
 
         variables.update(self._get_standard_current_variables(i_cc, i_boundary_cc))
 
+        variables.update(
+            {
+                "Composite negative current collector potential": phi_s_cn,
+                "Composite current collector current density": i_boundary_cc,
+            }
+        )
+
         return variables
 
 

pybamm/models/submodels/electrode/base_electrode.py

@@ -16,7 +16,7 @@ class BaseElectrode(pybamm.BaseSubModel):
     options : dict, optional
         A dictionary of options to be passed to the model.
     set_positive_potential :  bool, optional
-        If True the battery model sets the positve potential based on the current.
+        If True the battery model sets the positive potential based on the current.
         If False, the potential is specified by the user. Default is True.
 
     **Extends:** :class:`pybamm.BaseSubModel`

pybamm/models/submodels/electrode/ohm/composite_ohm.py

@@ -27,12 +27,13 @@ def __init__(self, param, domain, options=None):
         super().__init__(param, domain, options=options)
 
     def get_coupled_variables(self, variables):
+        param = self.param
 
         i_boundary_cc_0 = variables["Leading-order current collector current density"]
 
         # import parameters and spatial variables
-        l_n = self.param.l_n
-        l_p = self.param.l_p
+        l_n = param.l_n
+        l_p = param.l_p
         x_n = pybamm.standard_spatial_vars.x_n
         x_p = pybamm.standard_spatial_vars.x_p
 

pybamm/models/submodels/electrode/ohm/leading_ohm.py

@@ -37,12 +37,14 @@ def get_coupled_variables(self, variables):
         """
         Returns variables which are derived from the fundamental variables in the model.
         """
+        param = self.param
+
         i_boundary_cc = variables["Current collector current density"]
         phi_s_cn = variables["Negative current collector potential"]
 
         # import parameters and spatial variables
-        l_n = self.param.l_n
-        l_p = self.param.l_p
+        l_n = param.l_n
+        l_p = param.l_p
         x_n = pybamm.standard_spatial_vars.x_n
         x_p = pybamm.standard_spatial_vars.x_p