From faf0a59fb1baa22dfdb45903f49b3396a0670fab Mon Sep 17 00:00:00 2001 From: Scott Marquis Date: Fri, 8 Nov 2019 17:37:20 +0000 Subject: [PATCH] #692 changed to pass in external option --- pybamm/discretisations/discretisation.py | 14 ++- pybamm/models/base_model.py | 7 +- .../full_battery_models/base_battery_model.py | 98 ++++++++++++------- pybamm/models/submodels/base_submodel.py | 38 +++++-- .../submodels/thermal/external/__init__.py | 0 .../models/submodels/thermal/external/full.py | 32 ------ .../submodels/thermal/external/lumped.py | 39 -------- pybamm/simulation.py | 7 +- .../test_use_external_submodel.py | 25 +++++ .../standard_submodel_unit_tests.py | 6 ++ 10 files changed, 143 insertions(+), 123 deletions(-) delete mode 100644 pybamm/models/submodels/thermal/external/__init__.py delete mode 100644 pybamm/models/submodels/thermal/external/full.py delete mode 100644 pybamm/models/submodels/thermal/external/lumped.py create mode 100644 tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_use_external_submodel.py diff --git a/pybamm/discretisations/discretisation.py b/pybamm/discretisations/discretisation.py index 0c1ab0c6ab..632ae9d8c2 100644 --- a/pybamm/discretisations/discretisation.py +++ b/pybamm/discretisations/discretisation.py @@ -104,7 +104,11 @@ def process_model(self, model, inplace=True): # Prepare discretisation # set variables (we require the full variable not just id) - variables = list(model.rhs.keys()) + list(model.algebraic.keys()) + variables = ( + list(model.rhs.keys()) + + list(model.algebraic.keys()) + + model.external_variables + ) # Set the y split for variables pybamm.logger.info("Set variable slices for {}".format(model.name)) @@ -132,6 +136,8 @@ def process_model(self, model, inplace=True): model_disc.bcs = self.bcs + self.external_variables = model.external_variables + # Process initial condtions pybamm.logger.info("Discretise initial conditions for {}".format(model.name)) ics, concat_ics = self.process_initial_conditions(model) @@ -795,7 +801,11 @@ def _concatenate_in_order(self, var_eqn_dict, check_complete=False, sparse=False if check_complete: # Check keys from the given var_eqn_dict against self.y_slices ids = {v.id for v in unpacked_variables} - if ids != self.y_slices.keys(): + external_id = {v.id for v in self.external_variables} + y_slices_with_external_removed = set(self.y_slices.keys()).difference( + external_id + ) + if ids != y_slices_with_external_removed: given_variable_names = [v.name for v in var_eqn_dict.keys()] raise pybamm.ModelError( "Initial conditions are insufficient. Only " diff --git a/pybamm/models/base_model.py b/pybamm/models/base_model.py index 9d192af47a..ebaef339b6 100644 --- a/pybamm/models/base_model.py +++ b/pybamm/models/base_model.py @@ -371,7 +371,12 @@ def check_well_determined(self, post_discretisation): # If any variables in the equations don't appear in the keys then the model is # underdetermined vars_in_keys = vars_in_rhs_keys.union(vars_in_algebraic_keys) - extra_variables = vars_in_eqns.difference(vars_in_keys) + extra_variables_in_equations = vars_in_eqns.difference(vars_in_keys) + + # get ids of external variables + external_ids = {var.id for var in self.external_variables} + extra_variables = extra_variables_in_equations.difference(external_ids) + if extra_variables: raise pybamm.ModelError("model is underdetermined (too many variables)") diff --git a/pybamm/models/full_battery_models/base_battery_model.py b/pybamm/models/full_battery_models/base_battery_model.py index 6bd4a3b6d7..a560dc5e3f 100644 --- a/pybamm/models/full_battery_models/base_battery_model.py +++ b/pybamm/models/full_battery_models/base_battery_model.py @@ -57,6 +57,11 @@ class BaseBatteryModel(pybamm.BaseModel): only takes effect if "dimensionality" is 0. If "dimensionality" is 1 or 2 current collector effects are always included. Must be 'False' for lead-acid models. + * "external submodels" : list + A list of the submodels that you would like to supply an external + variable for instead of solving in PyBaMM. The entries of the lists + are strings that correspond to the submodel names in the keys + of `self.submodels`. **Extends:** :class:`pybamm.BaseModel` @@ -68,6 +73,7 @@ def __init__(self, options=None, name="Unnamed battery model"): self.set_standard_output_variables() self.submodels = {} self._built = False + self._built_fundamental_and_external = False @property def default_parameter_values(self): @@ -157,6 +163,7 @@ def options(self, extra_options): "particle": "Fickian diffusion", "thermal": "isothermal", "thermal current collector": False, + "external submodels": [] } options = default_options # any extra options overwrite the default options @@ -396,17 +403,7 @@ def set_standard_output_variables(self): {"y": var.y, "y [m]": var.y * L_y, "z": var.z, "z [m]": var.z * L_z} ) - def build_model(self): - - # Check if already built - if self._built: - raise pybamm.ModelError( - """Model already built. If you are adding a new submodel, try using - `model.update` instead.""" - ) - - pybamm.logger.info("Building {}".format(self.name)) - + def build_fundamental_and_external(self): # Get the fundamental variables for submodel_name, submodel in self.submodels.items(): pybamm.logger.debug( @@ -416,7 +413,11 @@ def build_model(self): ) self.variables.update(submodel.get_fundamental_variables()) - # Get any external variables + # set the submodels that are external + for sub in self.options["external submodels"]: + self.submodels[sub].external = True + + # Set any external variables self.external_variables = [] for submodel_name, submodel in self.submodels.items(): pybamm.logger.debug( @@ -424,12 +425,13 @@ def build_model(self): submodel_name, self.name ) ) - variables, external_variables = submodel.get_external_variables() + external_variables = submodel.get_external_variables() - self.variables.update(variables) self.external_variables += external_variables - # Get coupled variables + self._built_fundamental_and_external = 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 @@ -462,36 +464,56 @@ def build_model(self): # try setting coupled variables on next loop through pass + def build_model_equations(self): # Set model equations for submodel_name, submodel in self.submodels.items(): - pybamm.logger.debug( - "Setting rhs for {} submodel ({})".format(submodel_name, self.name) - ) + if submodel.external is False: + pybamm.logger.debug( + "Setting rhs for {} submodel ({})".format(submodel_name, self.name) + ) - submodel.set_rhs(self.variables) - pybamm.logger.debug( - "Setting algebraic for {} submodel ({})".format( - submodel_name, self.name + submodel.set_rhs(self.variables) + pybamm.logger.debug( + "Setting algebraic for {} submodel ({})".format( + submodel_name, self.name + ) ) - ) - submodel.set_algebraic(self.variables) - pybamm.logger.debug( - "Setting boundary conditions for {} submodel ({})".format( - submodel_name, self.name + submodel.set_algebraic(self.variables) + pybamm.logger.debug( + "Setting boundary conditions for {} submodel ({})".format( + submodel_name, self.name + ) ) - ) - submodel.set_boundary_conditions(self.variables) - pybamm.logger.debug( - "Setting initial conditions for {} submodel ({})".format( - submodel_name, self.name + submodel.set_boundary_conditions(self.variables) + pybamm.logger.debug( + "Setting initial conditions for {} submodel ({})".format( + submodel_name, self.name + ) ) + submodel.set_initial_conditions(self.variables) + submodel.set_events(self.variables) + pybamm.logger.debug( + "Updating {} submodel ({})".format(submodel_name, self.name) + ) + self.update(submodel) + + def build_model(self): + + # Check if already built + if self._built: + raise pybamm.ModelError( + """Model already built. If you are adding a new submodel, try using + `model.update` instead.""" ) - submodel.set_initial_conditions(self.variables) - submodel.set_events(self.variables) - pybamm.logger.debug( - "Updating {} submodel ({})".format(submodel_name, self.name) - ) - self.update(submodel) + + pybamm.logger.info("Building {}".format(self.name)) + + if self._built_fundamental_and_external is False: + self.build_fundamental_and_external() + + self.build_coupled_variables() + + self.build_model_equations() pybamm.logger.debug("Setting voltage variables") self.set_voltage_variables() diff --git a/pybamm/models/submodels/base_submodel.py b/pybamm/models/submodels/base_submodel.py index 25a1aaf7c1..07df5c1e3d 100644 --- a/pybamm/models/submodels/base_submodel.py +++ b/pybamm/models/submodels/base_submodel.py @@ -46,7 +46,7 @@ class BaseSubModel: symbols. """ - def __init__(self, param, domain=None, reactions=None): + def __init__(self, param, domain=None, reactions=None, external=False): super().__init__() self.param = param # Initialise empty variables (to avoid overwriting with 'None') @@ -62,6 +62,8 @@ def __init__(self, param, domain=None, reactions=None): self.set_domain_for_broadcast() self.reactions = reactions + self.external = external + @property def domain(self): return self._domain @@ -101,20 +103,40 @@ def get_fundamental_variables(self): """ return {} - def set_external_variables(self): + def get_external_variables(self): """ - A public method that creates and returns the variables in a submodel which are - suppled external to the model. + A public method that returns the variables in a submodel which are + supplied by an external source. Returns ------- - dict : - The variables created by the submodel which are independent of variables in - other submodels. list : A list of the external variables in the model. """ - return {}, [] + + external_variables = [] + list_of_vars = [] + + if self.external is True: + # look through all the variables in the submodel and get the + # variables which are state vectors + submodel_variables = self.get_fundamental_variables() + for var in submodel_variables.values(): + if isinstance(var, pybamm.Variable): + list_of_vars += [var] + elif isinstance(var, pybamm.Concatenation): + for child in var.children: + if isinstance(child, pybamm.Variable): + list_of_vars += [child] + + # remove duplicates + unique_ids = [] + for var in list_of_vars: + if var.id not in unique_ids: + external_variables += [var] + unique_ids += [var.id] + + return external_variables def get_coupled_variables(self, variables): """ diff --git a/pybamm/models/submodels/thermal/external/__init__.py b/pybamm/models/submodels/thermal/external/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/pybamm/models/submodels/thermal/external/full.py b/pybamm/models/submodels/thermal/external/full.py deleted file mode 100644 index 4c131ebbe3..0000000000 --- a/pybamm/models/submodels/thermal/external/full.py +++ /dev/null @@ -1,32 +0,0 @@ -# -# A class for full external thermal models -# -import pybamm - -from ..base_thermal import BaseThermal - - -class Full(BaseThermal): - """Class to link full external thermal submodels. - - Parameters - ---------- - param : parameter class - The parameters to use for this submodel - - - **Extends:** :class:`pybamm.thermal.BaseModel` - """ - - def __init__(self, param): - super().__init__(param) - - def get_external_variables(self): - T = pybamm.standard_variables.T - T_cn = pybamm.BoundaryValue(T, "left") - T_cp = pybamm.BoundaryValue(T, "right") - - variables = self._get_standard_fundamental_variables(T, T_cn, T_cp) - external_variables = [T] - - return variables, external_variables diff --git a/pybamm/models/submodels/thermal/external/lumped.py b/pybamm/models/submodels/thermal/external/lumped.py deleted file mode 100644 index 7da09c5c2b..0000000000 --- a/pybamm/models/submodels/thermal/external/lumped.py +++ /dev/null @@ -1,39 +0,0 @@ -# -# A class for external lumped thermal models -# -import pybamm - -from ..base_thermal import BaseThermal - - -class Full(BaseThermal): - """Class to link external lumped thermal submodels. - - Parameters - ---------- - param : parameter class - The parameters to use for this submodel - - - **Extends:** :class:`pybamm.thermal.BaseModel` - """ - - def __init__(self, param): - super().__init__(param) - - def get_external_variables(self): - T_x_av = pybamm.standard_variables.T_av - T_n = pybamm.PrimaryBroadcast(T_x_av, "negative electrode") - T_s = pybamm.PrimaryBroadcast(T_x_av, "separator") - T_p = pybamm.PrimaryBroadcast(T_x_av, "positive electrode") - T = pybamm.Concatenation(T_n, T_s, T_p) - - T_cn = T_x_av - T_cp = T_x_av - - variables = self._get_standard_fundamental_variables(T, T_cn, T_cp) - variables = self._get_standard_fundamental_variables(T, T_cn, T_cp) - - external_variables = [T_x_av] - - return variables, external_variables diff --git a/pybamm/simulation.py b/pybamm/simulation.py index 774a5750b8..5c6bf34a2a 100644 --- a/pybamm/simulation.py +++ b/pybamm/simulation.py @@ -33,7 +33,7 @@ def __init__( self._solver = solver or self._model.default_solver self._quick_plot_vars = quick_plot_vars - self.reset() + self.reset(update_model=False) def set_defaults(self): """ @@ -48,11 +48,12 @@ def set_defaults(self): self._solver = self._model.default_solver self._quick_plot_vars = None - def reset(self): + def reset(self, update_model=True): """ A method to reset a simulation back to its unprocessed state. """ - self.model = self._model_class(self._model_options) + if update_model: + self.model = self._model_class(self._model_options) self.geometry = copy.deepcopy(self._unprocessed_geometry) self._model_with_set_params = None self._built_model = None diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_use_external_submodel.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_use_external_submodel.py new file mode 100644 index 0000000000..aa2e03a3b0 --- /dev/null +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_use_external_submodel.py @@ -0,0 +1,25 @@ +# +# Tests for external submodels +# +import pybamm +import unittest + + +# only works for statevector inputs +class TestExternalSubmodel(unittest.TestCase): + def test_external_temperature(self): + + model_options = {"thermal": "x-full", "external submodels": ["thermal"]} + model = pybamm.lithium_ion.SPMe(model_options) + + sim = pybamm.Simulation(model) + sim.build() + + +if __name__ == "__main__": + print("Add -v for more debug output") + import sys + + if "-v" in sys.argv: + debug = True + unittest.main() diff --git a/tests/unit/test_models/test_submodels/standard_submodel_unit_tests.py b/tests/unit/test_models/test_submodels/standard_submodel_unit_tests.py index 033fb95d86..f0d45ae51f 100644 --- a/tests/unit/test_models/test_submodels/standard_submodel_unit_tests.py +++ b/tests/unit/test_models/test_submodels/standard_submodel_unit_tests.py @@ -14,10 +14,15 @@ def __init__(self, submodel, variables=None): variables = {} self.submodel = submodel self.variables = variables + self.external_variables = [] def test_get_fundamental_variables(self): self.variables.update(self.submodel.get_fundamental_variables()) + def test_get_external_variables(self): + external_variables = self.submodel.get_external_variables() + self.external_variables += external_variables + def test_get_coupled_variables(self): self.variables.update(self.submodel.get_coupled_variables(self.variables)) @@ -38,6 +43,7 @@ def test_set_events(self): def test_all(self): self.test_get_fundamental_variables() + self.test_get_external_variables() self.test_get_coupled_variables() self.test_set_rhs() self.test_set_algebraic()