Merge pull request #1974 from pybamm-team/remove-deprecated-experiment-inputs

valentinsulzer · web-flow · commit 795a0c081518 · 2022-03-10T12:04:36.000-05:00
remove deprecated stuff
diff --git a/pybamm/experiments/experiment.py b/pybamm/experiments/experiment.py
@@ -3,7 +3,6 @@
 #
 
 import numpy as np
-import warnings
 
 examples = """
 
@@ -43,17 +42,11 @@ class Experiment:
     ----------
     operating_conditions : list
         List of operating conditions
-    parameters : dict
-        Dictionary of parameters to use for this experiment, replacing default
-        parameters as appropriate
     period : string, optional
         Period (1/frequency) at which to record outputs. Default is 1 minute. Can be
         overwritten by individual operating conditions.
     termination : list, optional
         List of conditions under which to terminate the experiment. Default is None.
-    use_simulation_setup_type : str
-        Whether to use the "new" (default) or "old" simulation set-up type. "new" is
-        faster at simulating individual steps but has higher set-up overhead
     drive_cycles : dict
         Dictionary of drive cycles to use for this experiment.
     cccv_handling : str, optional
@@ -66,32 +59,14 @@ class Experiment:
     def __init__(
         self,
         operating_conditions,
-        parameters=None,
         period="1 minute",
         termination=None,
-        use_simulation_setup_type="new",
         drive_cycles={},
         cccv_handling="two-step",
     ):
         if cccv_handling not in ["two-step", "ode"]:
             raise ValueError("cccv_handling should be either 'two-step' or 'ode'")
         self.cccv_handling = cccv_handling
-        # Deprecations
-        if parameters is not None:
-            warnings.simplefilter("always", DeprecationWarning)
-            warnings.warn(
-                "'parameters' as an input to the Experiment class will soon be "
-                "deprecated. Please open an issue if you are using this feature.",
-                DeprecationWarning,
-            )
-        if use_simulation_setup_type == "old":
-            warnings.simplefilter("always", DeprecationWarning)
-            warnings.warn(
-                "'old' simulation setup type for the Experiment class will soon be "
-                "deprecated. Use 'new' instead. Please open an issue if this gives an "
-                "error or unexpected results.",
-                DeprecationWarning,
-            )
 
         self.period = self.convert_time_to_seconds(period.split())
         operating_conditions_cycles = []
@@ -147,15 +122,9 @@ def __init__(
         self.operating_conditions, self.events = self.read_operating_conditions(
             operating_conditions, drive_cycles
         )
-        parameters = parameters or {}
-        if isinstance(parameters, dict):
-            self.parameters = parameters
-        else:
-            raise TypeError("experimental parameters should be a dictionary")
 
         self.termination_string = termination
         self.termination = self.read_termination(termination)
-        self.use_simulation_setup_type = use_simulation_setup_type
 
     def __str__(self):
         return str(self.operating_conditions_strings)
diff --git a/pybamm/simulation.py b/pybamm/simulation.py
@@ -27,19 +27,6 @@ def is_notebook():
         return False  # Probably standard Python interpreter
 
 
-def constant_current_constant_voltage_constant_power(variables):
-    I = variables["Current [A]"]
-    V = variables["Battery voltage [V]"]
-    s_I = pybamm.InputParameter("Current switch")
-    s_V = pybamm.InputParameter("Voltage switch")
-    s_P = pybamm.InputParameter("Power switch")
-    return (
-        s_I * (I - pybamm.InputParameter("Current input [A]"))
-        + s_V * (V - pybamm.InputParameter("Voltage input [V]"))
-        + s_P * (V * I - pybamm.InputParameter("Power input [W]"))
-    )
-
-
 class Simulation:
     """A Simulation class for easy building and running of PyBaMM simulations.
 
@@ -86,8 +73,7 @@ def __init__(
         if isinstance(model, pybamm.lithium_ion.BasicDFNHalfCell):
             if experiment is not None:
                 raise NotImplementedError(
-                    "BasicDFNHalfCell is not compatible "
-                    "with experiment simulations yet."
+                    "BasicDFNHalfCell is not compatible with experiment simulations."
                 )
 
         if experiment is None:
@@ -156,8 +142,6 @@ def set_up_experiment(self, model, experiment):
 
         # Save the experiment
         self.experiment = experiment
-        # Update parameter values with experiment parameters
-        self._parameter_values.update(experiment.parameters)
         # Create a new submodel for each set of operating conditions and update
         # parameters and events accordingly
         self._experiment_inputs = []
@@ -172,7 +156,7 @@ def set_up_experiment(self, model, experiment):
                 "Voltage input [V]": 0,
                 "Power input [W]": 0,
             }
-            op_control = op["electric"][1]
+            op_units = op["electric"][1]
             if op["dc_data"] is not None:
                 # If operating condition includes a drive cycle, define the interpolant
                 timescale = self._parameter_values.evaluate(model.timescale)
@@ -181,28 +165,28 @@ def set_up_experiment(self, model, experiment):
                     op["dc_data"][:, 1],
                     timescale * (pybamm.t - pybamm.InputParameter("start time")),
                 )
-                if op_control == "A":
+                if op_units == "A":
                     operating_inputs.update(
                         {
                             "Current switch": 1,
                             "Current input [A]": drive_cycle_interpolant,
                         }
                     )
-                if op_control == "V":
+                if op_units == "V":
                     operating_inputs.update(
                         {
                             "Voltage switch": 1,
                             "Voltage input [V]": drive_cycle_interpolant,
                         }
                     )
-                if op_control == "W":
+                if op_units == "W":
                     operating_inputs.update(
                         {"Power switch": 1, "Power input [W]": drive_cycle_interpolant}
                     )
             else:
-                if op_control in ["A", "C"]:
+                if op_units in ["A", "C"]:
                     capacity = self._parameter_values["Nominal cell capacity [A.h]"]
-                    if op_control == "A":
+                    if op_units == "A":
                         I = op["electric"][0]
                         Crate = I / capacity
                     else:
@@ -211,7 +195,7 @@ def set_up_experiment(self, model, experiment):
                         I = Crate * capacity
                     if len(op["electric"]) == 4:
                         # Update inputs for CCCV
-                        op_control = "CCCV"  # change to CCCV
+                        op_units = "CCCV"  # change to CCCV
                         V = op["electric"][2]
                         operating_inputs.update(
                             {
@@ -225,13 +209,13 @@ def set_up_experiment(self, model, experiment):
                         operating_inputs.update(
                             {"Current switch": 1, "Current input [A]": I}
                         )
-                elif op_control == "V":
+                elif op_units == "V":
                     # Update inputs for constant voltage
                     V = op["electric"][0]
                     operating_inputs.update(
                         {"Voltage switch": 1, "Voltage input [V]": V}
                     )
-                elif op_control == "W":
+                elif op_units == "W":
                     # Update inputs for constant power
                     P = op["electric"][0]
                     operating_inputs.update({"Power switch": 1, "Power input [W]": P})
@@ -267,102 +251,21 @@ def set_up_experiment(self, model, experiment):
             # Add time to the experiment times
             dt = op["time"]
             if dt is None:
-                if op_control in ["A", "C", "CCCV"]:
+                if op_units in ["A", "C", "CCCV"]:
                     # Current control: max simulation time: 3 * max simulation time
                     # based on C-rate
                     dt = 3 / abs(Crate) * 3600  # seconds
-                    if op_control == "CCCV":
+                    if op_units == "CCCV":
                         dt *= 5  # 5x longer for CCCV
                 else:
                     # max simulation time: 1 day
                     dt = 24 * 3600  # seconds
             self._experiment_times.append(dt)
 
         # Set up model for experiment
-        if experiment.use_simulation_setup_type == "old":
-            self.set_up_model_for_experiment_old(model)
-        elif experiment.use_simulation_setup_type == "new":
-            self.set_up_model_for_experiment_new(model)
-
-    def set_up_model_for_experiment_old(self, model):
-        """
-        Set up self.model to be able to run the experiment (old version).
-        In this version, a single model is created which can then be called with
-        different inputs for current-control, voltage-control, or power-control.
+        self.set_up_model_for_experiment(model)
 
-        This reduces set-up time since only one model needs to be processed, but
-        increases simulation time since the model formulation is inefficient
-        """
-        # Create a new model where the current density is now a variable
-        # To do so, we replace all instances of the current density in the
-        # model with a current density variable, which is obtained from the
-        # FunctionControl submodel
-        # create the FunctionControl submodel and extract variables
-        external_circuit_variables = pybamm.external_circuit.FunctionControl(
-            model.param, None, model.options
-        ).get_fundamental_variables()
-
-        # Perform the replacement
-        symbol_replacement_map = {
-            model.variables[name]: variable
-            for name, variable in external_circuit_variables.items()
-        }
-        replacer = pybamm.SymbolReplacer(symbol_replacement_map)
-        new_model = replacer.process_model(model, inplace=False)
-
-        # Update the algebraic equation and initial conditions for FunctionControl
-        # This creates an algebraic equation for the current to allow current, voltage,
-        # or power control, together with the appropriate guess for the
-        # initial condition.
-        # External circuit submodels are always equations on the current
-        # The external circuit function should fix either the current, or the voltage,
-        # or a combination (e.g. I*V for power control)
-        i_cell = new_model.variables["Total current density"]
-        new_model.initial_conditions[i_cell] = new_model.param.current_with_time
-        new_model.algebraic[i_cell] = constant_current_constant_voltage_constant_power(
-            new_model.variables
-        )
-
-        # Remove upper and lower voltage cut-offs that are *not* part of the experiment
-        new_model.events = [
-            event
-            for event in model.events
-            if event.name not in ["Minimum voltage", "Maximum voltage"]
-        ]
-        # add current and voltage events to the model
-        # current events both negative and positive to catch specification
-        new_model.events.extend(
-            [
-                pybamm.Event(
-                    "Current cut-off (positive) [A] [experiment]",
-                    new_model.variables["Current [A]"]
-                    - abs(pybamm.InputParameter("Current cut-off [A]")),
-                ),
-                pybamm.Event(
-                    "Current cut-off (negative) [A] [experiment]",
-                    new_model.variables["Current [A]"]
-                    + abs(pybamm.InputParameter("Current cut-off [A]")),
-                ),
-                pybamm.Event(
-                    "Voltage cut-off [V] [experiment]",
-                    new_model.variables["Battery voltage [V]"]
-                    - pybamm.InputParameter("Voltage cut-off [V]"),
-                ),
-            ]
-        )
-
-        self.model = new_model
-
-        operating_conditions = set(
-            x["electric"] + (x["time"],) + (x["period"],)
-            for x in self.experiment.operating_conditions
-        )
-        self.op_conds_to_model_and_param = {
-            op_cond[:2]: (new_model, self.parameter_values)
-            for op_cond in operating_conditions
-        }
-
-    def set_up_model_for_experiment_new(self, model):
+    def set_up_model_for_experiment(self, model):
         """
         Set up self.model to be able to run the experiment (new version).
         In this version, a new model is created for each step.
@@ -612,19 +515,16 @@ def build_for_experiment(self, check_model=True):
                 unbuilt_model,
                 parameter_values,
             ) in self.op_conds_to_model_and_param.items():
-                if unbuilt_model in processed_models:
-                    built_model = processed_models[unbuilt_model]
-                else:
-                    # It's ok to modify the models in-place as they are not accessible
-                    # from outside the simulation
-                    model_with_set_params = parameter_values.process_model(
-                        unbuilt_model, inplace=True
-                    )
-                    built_model = self._disc.process_model(
-                        model_with_set_params, inplace=True, check_model=check_model
-                    )
-                    processed_models[unbuilt_model] = built_model
+                # It's ok to modify the models in-place as they are not accessible
+                # from outside the simulation
+                model_with_set_params = parameter_values.process_model(
+                    unbuilt_model, inplace=True
+                )
+                built_model = self._disc.process_model(
+                    model_with_set_params, inplace=True, check_model=check_model
+                )
 
+                processed_models[unbuilt_model] = built_model
                 self.op_conds_to_built_models[op_cond] = built_model
 
     def solve(
@@ -1205,23 +1105,6 @@ def output_variables(self, output_variables):
     def solution(self):
         return self._solution
 
-    def specs(
-        self,
-        geometry=None,
-        parameter_values=None,
-        submesh_types=None,
-        var_pts=None,
-        spatial_methods=None,
-        solver=None,
-        output_variables=None,
-        C_rate=None,
-    ):
-        "Deprecated method for setting specs"
-        raise NotImplementedError(
-            "The 'specs' method has been deprecated. "
-            "Create a new simulation for each different case instead."
-        )
-
     def save(self, filename):
         """Save simulation using pickle"""
         if self.model.convert_to_format == "python":
diff --git a/tests/unit/test_experiments/test_experiment.py b/tests/unit/test_experiments/test_experiment.py
@@ -38,7 +38,6 @@ def test_read_strings(self):
                 "Run US06 (V) for 5 minutes",
                 "Run US06 (W) for 0.5 hours",
             ],
-            {"test": "test"},
             drive_cycles={"US06": drive_cycle},
             period="20 seconds",
         )
@@ -135,7 +134,6 @@ def test_read_strings(self):
                 None,
             ],
         )
-        self.assertEqual(experiment.parameters, {"test": "test"})
         self.assertEqual(experiment.period, 20)
 
     def test_read_strings_cccv_combined(self):
@@ -326,10 +324,6 @@ def test_bad_strings(self):
             pybamm.Experiment(["Discharge at 1 B for 2 hours"])
         with self.assertRaisesRegex(ValueError, "time units must be"):
             pybamm.Experiment(["Discharge at 1 A for 2 years"])
-        with self.assertRaisesRegex(
-            TypeError, "experimental parameters should be a dictionary"
-        ):
-            pybamm.Experiment([], "not a dictionary")
 
     def test_termination(self):
         experiment = pybamm.Experiment(["Discharge at 1 C for 20 seconds"])
diff --git a/tests/unit/test_experiments/test_simulation_with_experiment.py b/tests/unit/test_experiments/test_simulation_with_experiment.py
@@ -176,23 +176,6 @@ def test_run_experiment_drive_cycle(self):
         self.assertIn(("drive_cycle", "V"), sim.op_conds_to_model_and_param)
         self.assertIn(("drive_cycle", "W"), sim.op_conds_to_model_and_param)
 
-    def test_run_experiment_old_setup_type(self):
-        experiment = pybamm.Experiment(
-            [
-                (
-                    "Discharge at C/20 for 1 hour",
-                    "Charge at 1 A until 4.1 V",
-                    "Hold at 4.1 V until C/2",
-                    "Discharge at 2 W for 1 hour",
-                ),
-            ],
-            use_simulation_setup_type="old",
-        )
-        model = pybamm.lithium_ion.SPM()
-        sim = pybamm.Simulation(model, experiment=experiment)
-        solution1 = sim.solve(solver=pybamm.CasadiSolver())
-        self.assertEqual(solution1.termination, "final time")
-
     def test_run_experiment_breaks_early(self):
         experiment = pybamm.Experiment(["Discharge at 2 C for 1 hour"])
         model = pybamm.lithium_ion.SPM()
diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_basic_models.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_basic_models.py
diff --git a/tests/unit/test_simulation.py b/tests/unit/test_simulation.py
