Issue 1011 model new copy

CHANGELOG.md

@@ -16,6 +16,8 @@
 
 ## Bug fixes
 
+-   Fixed `Simulation` to keep different copies of the model so that parameters can be changed between simulations ([#1090](https://github.com/pybamm-team/PyBaMM/pull/1090))
+-   Fixed `model.new_copy()` to keep custom submodels ([#1090](https://github.com/pybamm-team/PyBaMM/pull/1090))
 -   2D processed variables can now be evaluated at the domain boundaries ([#1088](https://github.com/pybamm-team/PyBaMM/pull/1088))
 -   Update the default variable points to better capture behaviour in the solid particles in li-ion models ([#1081](https://github.com/pybamm-team/PyBaMM/pull/1081))
 -   Fix `QuickPlot` to display variables discretised by FEM (in y-z) properly ([#1078](https://github.com/pybamm-team/PyBaMM/pull/1078))
@@ -26,6 +28,7 @@
 
 ## Breaking changes
 
+-   `Simulation.specs` and `Simulation.set_defaults` have been deprecated. Users should create a new `Simulation` object for each different case instead ([#1090](https://github.com/pybamm-team/PyBaMM/pull/1090))
 -  The solution times `t_eval` must now be provided to `Simulation.solve()` when not using an experiment or prescribing the current using drive cycle data ([#1086](https://github.com/pybamm-team/PyBaMM/pull/1086))
 
 # [v0.2.2](https://github.com/pybamm-team/PyBaMM/tree/v0.2.2) - 2020-06-01

docs/source/models/submodels/external_circuit/experiment_events.rst

@@ -0,0 +1,5 @@
+Experiment events
+=================================
+
+.. autoclass:: pybamm.external_circuit.ExperimentEvents
+    :members:

docs/source/models/submodels/external_circuit/index.rst

@@ -13,3 +13,4 @@ variable to be constant.
 
   current_control_external_circuit
   function_control_external_circuit
+  experiment_events

examples/notebooks/Getting Started/Tutorial 3 - Basic plotting.ipynb

@@ -746,68 +746,6 @@
     "sim.plot([[\"Electrode current density\", \"Electrolyte current density\"], \"Terminal voltage [V]\"])"
    ]
   },
-  {
-   "cell_type": "markdown",
-   "metadata": {},
-   "source": [
-    "#### Overwriting default plot variables"
-   ]
-  },
-  {
-   "cell_type": "markdown",
-   "metadata": {},
-   "source": [
-    "If you plan on plotting a specific set of variables repeatedly you can also overwrite the default set of quick plot variables in the simulation class. Currently, if we run `plot` with no arguments, we produces the standard plot we observed in previous tutorials:"
-   ]
-  },
-  {
-   "cell_type": "code",
-   "execution_count": 7,
-   "metadata": {},
-   "outputs": [
-    {
-     "data": {
-      "application/vnd.jupyter.widget-view+json": {
-       "model_id": "eb778fbfe8b8428d8bf63ce3142d36f0",
-       "version_major": 2,
-       "version_minor": 0
-      },
-      "text/plain": [
-       "interactive(children=(FloatSlider(value=0.0, description='t', max=1.0, step=0.01), Output()), _dom_classes=('w…"
-      ]
-     },
-     "metadata": {},
-     "output_type": "display_data"
-    }
-   ],
-   "source": [
-    "sim.plot()"
-   ]
-  },
-  {
-   "cell_type": "markdown",
-   "metadata": {},
-   "source": [
-    "But, if we set:"
-   ]
-  },
-  {
-   "cell_type": "code",
-   "execution_count": 8,
-   "metadata": {},
-   "outputs": [],
-   "source": [
-    "quick_plot_vars = [\"Electrolyte concentration [mol.m-3]\", \"Terminal voltage [V]\"]\n",
-    "sim.specs(quick_plot_vars=quick_plot_vars)"
-   ]
-  },
-  {
-   "cell_type": "markdown",
-   "metadata": {},
-   "source": [
-    "then running `plot` will automatically produce a plot using your plot variables:"
-   ]
-  },
   {
    "cell_type": "code",
    "execution_count": 9,
@@ -858,7 +796,7 @@
    "name": "python",
    "nbconvert_exporter": "python",
    "pygments_lexer": "ipython3",
-   "version": "3.6.9"
+   "version": "3.7.7"
   }
  },
  "nbformat": 4,

examples/notebooks/change-input-current.ipynb

@@ -338,7 +338,7 @@
    "name": "python",
    "nbconvert_exporter": "python",
    "pygments_lexer": "ipython3",
-   "version": "3.6.8"
+   "version": "3.7.7"
   }
  },
  "nbformat": 4,

pybamm/models/base_model.py

@@ -101,12 +101,12 @@ def __init__(self, name="Unnamed model"):
         self.options = {}
 
         # Initialise empty model
-        self.rhs = {}
-        self.algebraic = {}
-        self.initial_conditions = {}
-        self.boundary_conditions = {}
-        self.variables = {}
-        self.events = []
+        self._rhs = {}
+        self._algebraic = {}
+        self._initial_conditions = {}
+        self._boundary_conditions = {}
+        self._variables = {}
+        self._events = []
         self._concatenated_rhs = None
         self._concatenated_algebraic = None
         self._concatenated_initial_conditions = None
@@ -327,11 +327,12 @@ def _find_input_parameters(self):
     def __getitem__(self, key):
         return self.rhs[key]
 
-    def new_copy(self, options=None):
-        "Create an empty copy with identical options, or new options if specified"
-        options = options or self.options
-        new_model = self.__class__(options)
-        new_model.name = self.name
+    def new_copy(self, build=False):
+        """
+        Create an empty copy with identical options, or new options if specified.
+        The 'build' parameter is included for compatibility with subclasses, but unused.
+        """
+        new_model = self.__class__(name=self.name)
         new_model.use_jacobian = self.use_jacobian
         new_model.use_simplify = self.use_simplify
         new_model.convert_to_format = self.convert_to_format
@@ -349,7 +350,6 @@ def update(self, *submodels):
             The submodels from which to create new model
         """
         for submodel in submodels:
-
             # check and then update dicts
             self.check_and_combine_dict(self._rhs, submodel.rhs)
             self.check_and_combine_dict(self._algebraic, submodel.algebraic)
@@ -394,6 +394,7 @@ def check_well_posedness(self, post_discretisation=False):
         self.check_algebraic_equations(post_discretisation)
         self.check_ics_bcs()
         self.check_default_variables_dictionaries()
+        self.check_no_repeated_keys()
         # Can't check variables after discretising, since Variable objects get replaced
         # by StateVector objects
         # Checking variables is slow, so only do it in debug mode
@@ -622,6 +623,21 @@ def check_variables(self):
                     )
                 )
 
+    def check_no_repeated_keys(self):
+        "Check that no equation keys are repeated"
+        rhs_alg = {**self.rhs, **self.algebraic}
+        rhs_alg_keys = []
+
+        for var in rhs_alg.keys():
+            # Check the variable has not already been defined
+            if var.id in rhs_alg_keys:
+                raise pybamm.ModelError(
+                    "Multiple equations specified for variable {!r}".format(var)
+                )
+            # Update list of variables
+            else:
+                rhs_alg_keys.append(var.id)
+
     def info(self, symbol_name):
         """
         Provides helpful summary information for a symbol.

pybamm/models/full_battery_models/base_battery_model.py

@@ -489,12 +489,14 @@ def build_model_equations(self):
                         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(
@@ -507,6 +509,7 @@ def build_model_equations(self):
                     "Updating {} submodel ({})".format(submodel_name, self.name)
                 )
                 self.update(submodel)
+                self.check_no_repeated_keys()
 
     def build_model(self):
 
@@ -545,6 +548,36 @@ def build_model(self):
 
         self._built = True
 
+    def new_copy(self, build=True):
+        """
+        Create a copy of the model. Overwrites the functionality of
+        :class:`pybamm.BaseModel` to make sure that the submodels are updated correctly
+        """
+        # create without building
+        # 'build' is not a keyword argument for the BaseBatteryModel class, but it
+        # should be for all of the subclasses
+        new_model = self.__class__(options=self.options, name=self.name, build=False)
+        # update submodels
+        new_model.submodels = self.submodels
+        # clear submodel equations to avoid weird conflicts
+        for submodel in self.submodels.values():
+            submodel._rhs = {}
+            submodel._algebraic = {}
+            submodel._initial_conditions = {}
+            submodel._boundary_conditions = {}
+            submodel._variables = {}
+            submodel._events = []
+
+        # now build
+        if build:
+            new_model.build_model()
+        new_model.use_jacobian = self.use_jacobian
+        new_model.use_simplify = self.use_simplify
+        new_model.convert_to_format = self.convert_to_format
+        new_model.timescale = self.timescale
+        new_model.length_scales = self.length_scales
+        return new_model
+
     def set_external_circuit_submodel(self):
         """
         Define how the external circuit defines the boundary conditions for the model,

pybamm/models/full_battery_models/lead_acid/basic_full.py

@@ -298,3 +298,6 @@ def __init__(self, name="Basic full model"):
                 pybamm.Event("Maximum voltage", voltage - param.voltage_high_cut),
             ]
         )
+
+    def new_copy(self, build=False):
+        return pybamm.BaseModel.new_copy(self)

pybamm/models/full_battery_models/lithium_ion/basic_dfn.py

@@ -282,3 +282,6 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
             pybamm.Event("Minimum voltage", voltage - param.voltage_low_cut),
             pybamm.Event("Maximum voltage", voltage - param.voltage_high_cut),
         ]
+
+    def new_copy(self, build=False):
+        return pybamm.BaseModel.new_copy(self)

pybamm/models/full_battery_models/lithium_ion/basic_spm.py

@@ -167,3 +167,7 @@ def __init__(self, name="Single Particle Model"):
             pybamm.Event("Minimum voltage", V - param.voltage_low_cut),
             pybamm.Event("Maximum voltage", V - param.voltage_high_cut),
         ]
+
+    def new_copy(self, build=False):
+        return pybamm.BaseModel.new_copy(self)
+

pybamm/models/submodels/external_circuit/__init__.py

@@ -8,3 +8,4 @@
     LeadingOrderVoltageFunctionControl,
     LeadingOrderPowerFunctionControl,
 )
+from .experiment_events import ExperimentEvents

pybamm/models/submodels/external_circuit/experiment_events.py

@@ -0,0 +1,35 @@
+#
+# Model to impose the events for experiments
+#
+import pybamm
+
+
+class ExperimentEvents(pybamm.BaseSubModel):
+    """Model to impose the events for experiments."""
+
+    def __init__(self, param):
+        super().__init__(param)
+
+    def set_events(self, variables):
+        # add current and voltage events to the model
+        # current events both negative and positive to catch specification
+        n_cells = pybamm.electrical_parameters.n_cells
+        self.events.extend(
+            [
+                pybamm.Event(
+                    "Current cut-off (positive) [A] [experiment]",
+                    variables["Current [A]"]
+                    - abs(pybamm.InputParameter("Current cut-off [A]")),
+                ),
+                pybamm.Event(
+                    "Current cut-off (negative) [A] [experiment]",
+                    variables["Current [A]"]
+                    + abs(pybamm.InputParameter("Current cut-off [A]")),
+                ),
+                pybamm.Event(
+                    "Voltage cut-off [V] [experiment]",
+                    variables["Terminal voltage [V]"]
+                    - pybamm.InputParameter("Voltage cut-off [V]") / n_cells,
+                ),
+            ]
+        )