reformat options to avoid errors with custom models

CHANGELOG.md

@@ -12,6 +12,7 @@
 
 ## Bug fixes
 
+- Allow models that subclass `BaseBatteryModel` to use custom options classes ([#2571](https://github.com/pybamm-team/PyBaMM/pull/2571))
 - Fixed bug with `EntryPoints` in Spyder IDE ([#2584](https://github.com/pybamm-team/PyBaMM/pull/2584))
 - Fixed electrolyte conservation when options {"surface form": "algebraic"} are used
 - Fixed "constant concentration" electrolyte model so that "porosity times concentration" is conserved when porosity changes ([#2529](https://github.com/pybamm-team/PyBaMM/pull/2529))

pybamm/geometry/battery_geometry.py

@@ -7,7 +7,6 @@
 def battery_geometry(
     include_particles=True,
     options=None,
-    current_collector_dimension=0,
     form_factor="pouch",
 ):
     """
@@ -20,9 +19,6 @@ def battery_geometry(
     options : dict, optional
         Dictionary of model options. Necessary for "particle-size geometry",
         relevant for lithium-ion chemistries.
-    current_collector_dimensions : int, optional
-        The dimensions of the current collector. Can be 0 (default), 1 or 2. For
-        a "cylindrical" form factor the current collector dimension must be 0 or 1.
     form_factor : str, optional
         The form factor of the cell. Can be "pouch" (default) or "cylindrical".
 
@@ -32,7 +28,9 @@ def battery_geometry(
         A geometry class for the battery
 
     """
-    options = pybamm.BatteryModelOptions(options or {})
+    if options is None or type(options) == dict:
+        options = pybamm.BatteryModelOptions(options)
+
     geo = pybamm.geometric_parameters
     l_n = geo.n.l
     l_s = geo.s.l
@@ -77,6 +75,7 @@ def battery_geometry(
             }
         )
     # Add current collector domains
+    current_collector_dimension = options["dimensionality"]
     if form_factor == "pouch":
         if current_collector_dimension == 0:
             geometry["current collector"] = {"z": {"position": 1}}
@@ -105,12 +104,6 @@ def battery_geometry(
                     },
                 },
             }
-        else:
-            raise pybamm.GeometryError(
-                "Invalid current collector dimension '{}' (should be 0, 1 or 2)".format(
-                    current_collector_dimension
-                )
-            )
     elif form_factor == "cylindrical":
         if current_collector_dimension == 0:
             geometry["current collector"] = {"r_macro": {"position": 1}}

pybamm/models/full_battery_models/base_battery_model.py

@@ -686,6 +686,19 @@ def __getitem__(self, key):
 class BaseBatteryModel(pybamm.BaseModel):
     """
     Base model class with some default settings and required variables
+
+    Parameters
+    ----------
+    options : dict-like, optional
+        A dictionary of options to be passed to the model. If this is a dict (and not
+        a subtype of dict), it will be processed by :class:`pybamm.BatteryModelOptions`
+        to ensure that the options are valid. If this is a subtype of dict, it is
+        assumed that the options have already been processed and are valid. This allows
+        for the use of custom options classes. The default options are given by
+        :class:`pybamm.BatteryModelOptions`.
+    name : str, optional
+        The name of the model. The default is "Unnamed battery model".
+
     **Extends:** :class:`pybamm.BaseModel`
     """
 
@@ -710,10 +723,7 @@ def length_scales(self, value):
 
     @property
     def default_geometry(self):
-        return pybamm.battery_geometry(
-            options=self.options,
-            current_collector_dimension=self.options["dimensionality"],
-        )
+        return pybamm.battery_geometry(options=self.options)
 
     @property
     def default_var_pts(self):
@@ -791,7 +801,13 @@ def options(self):
 
     @options.setter
     def options(self, extra_options):
-        options = BatteryModelOptions(extra_options)
+        # if extra_options is a dict then process it into a BatteryModelOptions
+        # this does not catch cases that subclass the dict type
+        # so other submodels can pass in their own options class if needed
+        if extra_options is None or type(extra_options) == dict:
+            options = BatteryModelOptions(extra_options)
+        else:
+            options = extra_options
 
         # Options that are incompatible with models
         if isinstance(self, pybamm.lithium_ion.BaseModel):
@@ -884,62 +900,6 @@ def set_standard_output_variables(self):
                 {"y": var.y, "y [m]": var.y * L_z, "z": var.z, "z [m]": var.z * L_z}
             )
 
-    def build_fundamental(self):
-        # Get the fundamental variables
-        for submodel_name, submodel in self.submodels.items():
-            pybamm.logger.debug(
-                "Getting fundamental variables for {} submodel ({})".format(
-                    submodel_name, self.name
-                )
-            )
-            self.variables.update(submodel.get_fundamental_variables())
-
-        self._built_fundamental = True
-
-    def build_coupled_variables(self):
-        # Note: pybamm will try to get the coupled variables for the submodels in the
-        # order they are set by the user. If this fails for a particular submodel,
-        # return to it later and try again. If setting coupled variables fails and
-        # there are no more submodels to try, raise an error.
-        submodels = list(self.submodels.keys())
-        count = 0
-        # For this part the FuzzyDict of variables is briefly converted back into a
-        # normal dictionary for speed with KeyErrors
-        self._variables = dict(self._variables)
-        while len(submodels) > 0:
-            count += 1
-            for submodel_name, submodel in self.submodels.items():
-                if submodel_name in submodels:
-                    pybamm.logger.debug(
-                        "Getting coupled variables for {} submodel ({})".format(
-                            submodel_name, self.name
-                        )
-                    )
-                    try:
-                        self.variables.update(
-                            submodel.get_coupled_variables(self.variables)
-                        )
-                        submodels.remove(submodel_name)
-                    except KeyError as key:
-                        if len(submodels) == 1 or count == 100:
-                            # no more submodels to try
-                            raise pybamm.ModelError(
-                                "Missing variable for submodel '{}': {}.\n".format(
-                                    submodel_name, key
-                                )
-                                + "Check the selected "
-                                "submodels provide all of the required variables."
-                            )
-                        else:
-                            # try setting coupled variables on next loop through
-                            pybamm.logger.debug(
-                                "Can't find {}, trying other submodels first".format(
-                                    key
-                                )
-                            )
-        # Convert variables back into FuzzyDict
-        self.variables = pybamm.FuzzyDict(self._variables)
-
     def build_model_equations(self):
         # Set model equations
         for submodel_name, submodel in self.submodels.items():

pybamm/models/full_battery_models/equivalent_circuit/thevenin.py

@@ -111,15 +111,7 @@ def set_options(self, extra_options=None):
                     )
                 )
 
-        self.ecm_options = options
-
-        # Hack to deal with submodels requiring electrochemical model
-        # options
-        self.options = pybamm.BatteryModelOptions({})
-        self.options["calculate discharge energy"] = self.ecm_options[
-            "calculate discharge energy"
-        ]
-        self.options["operating mode"] = self.ecm_options["operating mode"]
+        self.options = options
 
     def set_external_circuit_submodel(self):
         """
@@ -167,34 +159,34 @@ def set_external_circuit_submodel(self):
     def set_ocv_submodel(self):
         self.submodels[
             "Open circuit voltage"
-        ] = pybamm.equivalent_circuit_elements.OCVElement(self.param, self.ecm_options)
+        ] = pybamm.equivalent_circuit_elements.OCVElement(self.param, self.options)
 
     def set_resistor_submodel(self):
 
         name = "Element-0 (Resistor)"
         self.submodels[name] = pybamm.equivalent_circuit_elements.ResistorElement(
-            self.param, self.ecm_options
+            self.param, self.options
         )
         self.element_counter += 1
 
     def set_rc_submodels(self):
-        number_of_rc_elements = self.ecm_options["number of rc elements"]
+        number_of_rc_elements = self.options["number of rc elements"]
 
         for _ in range(number_of_rc_elements):
             name = f"Element-{self.element_counter} (RC)"
             self.submodels[name] = pybamm.equivalent_circuit_elements.RCElement(
-                self.param, self.element_counter, self.ecm_options
+                self.param, self.element_counter, self.options
             )
             self.element_counter += 1
 
     def set_thermal_submodel(self):
         self.submodels["Thermal"] = pybamm.equivalent_circuit_elements.ThermalSubModel(
-            self.param, self.ecm_options
+            self.param, self.options
         )
 
     def set_voltage_submodel(self):
         self.submodels["Voltage"] = pybamm.equivalent_circuit_elements.VoltageModel(
-            self.param, self.ecm_options
+            self.param, self.options
         )
 
     def set_submodels(self, build):

pybamm/models/full_battery_models/lead_acid/base_lead_acid_model.py

@@ -10,14 +10,30 @@ class BaseModel(pybamm.BaseBatteryModel):
     Overwrites default parameters from Base Model with default parameters for
     lead-acid models
 
+    Parameters
+    ----------
+    options : dict-like, optional
+        A dictionary of options to be passed to the model. If this is a dict (and not
+        a subtype of dict), it will be processed by :class:`pybamm.BatteryModelOptions`
+        to ensure that the options are valid. If this is a subtype of dict, it is
+        assumed that the options have already been processed and are valid. This allows
+        for the use of custom options classes. The default options are given by
+        :class:`pybamm.BatteryModelOptions`.
+    name : str, optional
+        The name of the model. The default is "Unnamed battery model".
+    build : bool, optional
+        Whether to build the model on instantiation. Default is True. Setting this
+        option to False allows users to change any number of the submodels before
+        building the complete model (submodels cannot be changed after the model is
+        built).
 
     **Extends:** :class:`pybamm.BaseBatteryModel`
 
     """
 
     def __init__(self, options=None, name="Unnamed lead-acid model", build=False):
         options = options or {}
-        # Specify that there are no particles in lead-acid, and no half-cell models
+        # Specify that there are no particles in lead-acid
         options["particle shape"] = "no particles"
         super().__init__(options, name)
         self.param = pybamm.LeadAcidParameters()
@@ -42,10 +58,7 @@ def default_parameter_values(self):
 
     @property
     def default_geometry(self):
-        return pybamm.battery_geometry(
-            include_particles=False,
-            current_collector_dimension=self.options["dimensionality"],
-        )
+        return pybamm.battery_geometry(include_particles=False, options=self.options)
 
     @property
     def default_var_pts(self):

pybamm/models/full_battery_models/lead_acid/full.py

@@ -9,19 +9,8 @@ class Full(BaseModel):
     """
     Porous electrode model for lead-acid, from [1]_, based on the Newman-Tiedemann
     model.
+    See :class:`pybamm.lead_acid.BaseModel` for more details.
 
-    Parameters
-    ----------
-    options : dict, optional
-        A dictionary of options to be passed to the model. For a detailed list of
-        options see :class:`~pybamm.BatteryModelOptions`.
-    name : str, optional
-        The name of the model.
-    build :  bool, optional
-        Whether to build the model on instantiation. Default is True. Setting this
-        option to False allows users to change any number of the submodels before
-        building the complete model (submodels cannot be changed after the model is
-        built).
 
     References
     ----------

pybamm/models/full_battery_models/lead_acid/loqs.py

@@ -8,19 +8,7 @@
 class LOQS(BaseModel):
     """
     Leading-Order Quasi-Static model for lead-acid, from [1]_.
-
-    Parameters
-    ----------
-    options : dict, optional
-        A dictionary of options to be passed to the model. For a detailed list of
-        options see :class:`~pybamm.BatteryModelOptions`.
-    name : str, optional
-        The name of the model.
-    build :  bool, optional
-        Whether to build the model on instantiation. Default is True. Setting this
-        option to False allows users to change any number of the submodels before
-        building the complete model (submodels cannot be changed after the model is
-        built).
+    See :class:`pybamm.lead_acid.BaseModel` for more details.
 
     References
     ----------

pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py

@@ -9,13 +9,30 @@ class BaseModel(pybamm.BaseBatteryModel):
     Overwrites default parameters from Base Model with default parameters for
     lithium-ion models
 
+    Parameters
+    ----------
+    options : dict-like, optional
+        A dictionary of options to be passed to the model. If this is a dict (and not
+        a subtype of dict), it will be processed by :class:`pybamm.BatteryModelOptions`
+        to ensure that the options are valid. If this is a subtype of dict, it is
+        assumed that the options have already been processed and are valid. This allows
+        for the use of custom options classes. The default options are given by
+        :class:`pybamm.BatteryModelOptions`.
+    name : str, optional
+        The name of the model. The default is "Unnamed battery model".
+    build : bool, optional
+        Whether to build the model on instantiation. Default is True. Setting this
+        option to False allows users to change any number of the submodels before
+        building the complete model (submodels cannot be changed after the model is
+        built).
+
     **Extends:** :class:`pybamm.BaseBatteryModel`
 
     """
 
     def __init__(self, options=None, name="Unnamed lithium-ion model", build=False):
         super().__init__(options, name)
-        self.param = pybamm.LithiumIonParameters(options)
+        self.param = pybamm.LithiumIonParameters(self.options)
 
         # Default timescale
         self._timescale = self.param.timescale

pybamm/models/full_battery_models/lithium_ion/dfn.py

@@ -8,19 +8,8 @@
 class DFN(BaseModel):
     """
     Doyle-Fuller-Newman (DFN) model of a lithium-ion battery, from [1]_.
+    See :class:`pybamm.lithium_ion.BaseModel` for more details.
 
-    Parameters
-    ----------
-    options : dict, optional
-        A dictionary of options to be passed to the model. For a detailed list of
-        options see :class:`~pybamm.BatteryModelOptions`.
-    name : str, optional
-        The name of the model.
-    build :  bool, optional
-        Whether to build the model on instantiation. Default is True. Setting this
-        option to False allows users to change any number of the submodels before
-        building the complete model (submodels cannot be changed after the model is
-        built).
     Examples
     --------
     >>> import pybamm

pybamm/models/full_battery_models/lithium_ion/mpm.py

@@ -9,19 +9,8 @@ class MPM(SPM):
     """
     Many-Particle Model (MPM) of a lithium-ion battery with particle-size
     distributions for each electrode, from [1]_.
+    See :class:`pybamm.lithium_ion.BaseModel` for more details.
 
-    Parameters
-    ----------
-    options : dict, optional
-        A dictionary of options to be passed to the model. For a detailed list of
-        options see :class:`~pybamm.BatteryModelOptions`.
-    name : str, optional
-        The name of the model.
-    build :  bool, optional
-        Whether to build the model on instantiation. Default is True. Setting this
-        option to False allows users to change any number of the submodels before
-        building the complete model (submodels cannot be changed after the model is
-        built).
     Examples
     --------
     >>> import pybamm