Repository: pybamm-team/PyBaMM

Issue:
update_model doesn't catch param in current function
When updating the parameters using `update_model`, the parameters used in the current functions aren't updated properly 


Discussion:

Patch:
diff --git a/examples/notebooks/change-input-current.ipynb b/examples/notebooks/change-input-current.ipynb
index 0353eab33..22349df65 100644
--- a/examples/notebooks/change-input-current.ipynb
+++ b/examples/notebooks/change-input-current.ipynb
@@ -66,7 +66,7 @@
    "outputs": [],
    "source": [
     "# update the value of the current profile *without* changing the typical current used for non-dimensionlisation\n",
-    "param[\"Current function\"] = pybamm.GetConstantCurrent(current=8)"
+    "param[\"Current function\"] = pybamm.GetConstantCurrent(current=pybamm.Scalar(8))"
    ]
   },
   {
@@ -84,7 +84,7 @@
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "0b36028f505f446fa15e3ab816c1e5ce",
+       "model_id": "fc997d14787542f69f2cba123ce0b994",
        "version_major": 2,
        "version_minor": 0
       },
@@ -175,14 +175,14 @@
      "name": "stderr",
      "output_type": "stream",
      "text": [
-      "/home/user/Documents/PyBaMM/pybamm/parameters/standard_current_functions/get_current_data.py:85: ModelWarning: Requested time (2387.4040040888344) is outside of the data range [0, 600]\n",
+      "/home/user/Documents/PyBaMM/pybamm/parameters/standard_current_functions/get_current_data.py:88: ModelWarning: Requested time (2387.404004088835) is outside of the data range [0, 600]\n",
       "  pybamm.ModelWarning,\n"
      ]
     },
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "40f34e811e9b43209224c4b829edb5c1",
+       "model_id": "3f4c99601ab8417ba2186987cb532600",
        "version_major": 2,
        "version_minor": 0
       },
@@ -312,7 +312,7 @@
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "91994467f3494f8fa63097786896af22",
+       "model_id": "913efe4e3e9e4b37bad2ded9c71b975e",
        "version_major": 2,
        "version_minor": 0
       },
diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py
index 3c23f2c59..5866734be 100644
--- a/pybamm/parameters/parameter_values.py
+++ b/pybamm/parameters/parameter_values.py
@@ -329,15 +329,19 @@ class ParameterValues(dict):
                     # KeyError -> name not in parameter dict, don't update
                     continue
             elif isinstance(x, pybamm.Function):
-                # Need to update values in parameters_eval dict of current functions
                 if isinstance(x.function, pybamm.GetCurrent):
+                    # Need to update parameters dict to be that of the new current
+                    # function and make new parameters_eval dict to be processed
+                    x.function.parameters = self["Current function"].parameters
+                    x.function.parameters_eval = x.function.parameters.copy()
                     for param, sym in x.function.parameters.items():
-                        if isinstance(sym, pybamm.Scalar):
-                            try:
-                                x.function.parameters_eval[param] = self[sym.name]
-                            except KeyError:
-                                # KeyError -> name not in parameter dict, don't update
-                                continue
+                        # Need to process again as new symbols may be passed
+                        # e.g. may explicitly pass pybamm.Scalar(1) instead of
+                        # pybamm.electrical_parameters.I_typ
+                        if isinstance(sym, pybamm.Symbol):
+                            new_sym = self.process_symbol(sym)
+                            x.function.parameters[param] = new_sym
+                            x.function.parameters_eval[param] = new_sym.evaluate()
                     if isinstance(x.function, pybamm.GetCurrentData):
                         # update interpolant
                         x.function.interpolate()
diff --git a/pybamm/parameters/standard_current_functions/get_current_data.py b/pybamm/parameters/standard_current_functions/get_current_data.py
index 53bbc9c53..d683120ea 100644
--- a/pybamm/parameters/standard_current_functions/get_current_data.py
+++ b/pybamm/parameters/standard_current_functions/get_current_data.py
@@ -21,7 +21,7 @@ class GetCurrentData(pybamm.GetCurrent):
     units : str, optional
         The units of the current data which is to be loaded. Can be "[]" for
         dimenionless data (default), or "[A]" for current in Amperes.
-    current_cale : :class:`pybamm.Symbol` or float, optional
+    current_scale : :class:`pybamm.Symbol` or float, optional
         The scale the current in Amperes if loading non-dimensional data. Default
         is to use the typical current I_typ
 
diff --git a/results/drive_cycles/discharge_rest.py b/results/drive_cycles/discharge_rest.py
new file mode 100644
index 000000000..ad3950c64
--- /dev/null
+++ b/results/drive_cycles/discharge_rest.py
@@ -0,0 +1,85 @@
+#
+# Simulate discharge followed by rest
+#
+import pybamm
+import numpy as np
+import matplotlib.pyplot as plt
+
+# load model
+pybamm.set_logging_level("INFO")
+model = pybamm.lithium_ion.SPMe()
+
+# create geometry
+geometry = model.default_geometry
+
+# load parameter values and process model and geometry
+param = model.default_parameter_values
+# adjust current to correspond to a typical current density of 24 [A.m-2]
+C_rate = 1
+param["Typical current [A]"] = (
+    C_rate * 24 * param.process_symbol(pybamm.geometric_parameters.A_cc).evaluate()
+)
+param.process_model(model)
+param.process_geometry(geometry)
+
+# set mesh
+mesh = pybamm.Mesh(geometry, model.default_submesh_types, model.default_var_pts)
+
+# discretise model
+disc = pybamm.Discretisation(mesh, model.default_spatial_methods)
+disc.process_model(model)
+
+# solve model during discharge stage (1 hour)
+tau = param.process_symbol(pybamm.standard_parameters_lithium_ion.tau_discharge)
+t_end = 3600 / tau.evaluate(0)
+t_eval1 = np.linspace(0, t_end, 120)
+solution1 = model.default_solver.solve(model, t_eval1)
+
+# process variables for later plotting
+time1 = pybamm.ProcessedVariable(
+    model.variables["Time [h]"], solution1.t, solution1.y
+)
+voltage1 = pybamm.ProcessedVariable(
+    model.variables["Terminal voltage [V]"], solution1.t, solution1.y, mesh=mesh
+)
+current1 = pybamm.ProcessedVariable(
+    model.variables["Current [A]"], solution1.t, solution1.y, mesh=mesh
+)
+
+# solve again with zero current, using last step of solution1 as initial conditions
+# update the current to be zero
+param["Current function"] = pybamm.GetConstantCurrent(current=pybamm.Scalar(0))
+param.update_model(model, disc)
+# Note: need to update model.concatenated_initial_conditions *after* update_model,
+# as update_model updates model.concatenated_initial_conditions, by concatenting
+# the (unmodified) initial conditions for each variable
+model.concatenated_initial_conditions = solution1.y[:, -1][:, np.newaxis]
+
+# simulate 1 hour of rest
+t_start = solution1.t[-1]
+t_end = t_start + 3600 / tau.evaluate(0)
+t_eval2 = np.linspace(t_start, t_end, 120)
+solution2 = model.default_solver.solve(model, t_eval2)
+
+# process variables for later plotting
+time2 = pybamm.ProcessedVariable(
+    model.variables["Time [h]"], solution2.t, solution2.y
+)
+voltage2 = pybamm.ProcessedVariable(
+    model.variables["Terminal voltage [V]"], solution2.t, solution2.y, mesh=mesh
+)
+current2 = pybamm.ProcessedVariable(
+    model.variables["Current [A]"], solution2.t, solution2.y, mesh=mesh
+)
+
+# plot
+plt.subplot(121)
+plt.plot(time1(t_eval1), voltage1(t_eval1), time2(t_eval2), voltage2(t_eval2))
+plt.xlabel('Time [h]')
+plt.ylabel('Voltage [V]')
+plt.subplot(122)
+z = np.linspace(0, 1, 10)
+plt.plot(time1(t_eval1), current1(t_eval1), time2(t_eval2), current2(t_eval2))
+plt.xlabel('Time [h]')
+plt.ylabel('Current [A]')
+plt.show()


Test patch:
diff --git a/tests/unit/test_parameters/test_update_parameters.py b/tests/unit/test_parameters/test_update_parameters.py
index 2b4a8b60e..7707a9517 100644
--- a/tests/unit/test_parameters/test_update_parameters.py
+++ b/tests/unit/test_parameters/test_update_parameters.py
@@ -60,6 +60,33 @@ class TestUpdateParameters(unittest.TestCase):
         # results should be different
         self.assertNotEqual(np.linalg.norm(Y1 - Y2), 0)
 
+        # test with new current function
+        model3 = pybamm.ReactionDiffusionModel()
+        modeltest3 = tests.StandardModelTest(model3)
+        modeltest3.test_all()
+        parameter_values_update = pybamm.ParameterValues(
+            base_parameters=model3.default_parameter_values,
+            optional_parameters={
+                "Current function": pybamm.GetConstantCurrent(current=pybamm.Scalar(0))
+            },
+        )
+        modeltest3.test_update_parameters(parameter_values_update)
+        modeltest3.test_solving(t_eval=t_eval)
+        Y3 = modeltest3.solution.y
+
+        # function.parameters should be pybamm.Scalar(0), but parameters_eval s
+        # should be a float
+        self.assertIsInstance(
+            model3.variables["Current [A]"].function.parameters["Current [A]"],
+            pybamm.Scalar,
+        )
+        self.assertEqual(
+            model3.variables["Current [A]"].function.parameters_eval["Current [A]"], 0.0
+        )
+
+        # results should be different
+        self.assertNotEqual(np.linalg.norm(Y1 - Y3), 0)
+
     def test_update_geometry(self):
         # standard model
         model1 = pybamm.ReactionDiffusionModel()