Removing old arguments (#3682)

* Removing old arguments * A couple extra fixes * Removing more old warnings, tests failing * Remove old tests * Extract method and add a test * Remove parameters CLI and associated files --------- Co-authored-by: Agriya Khetarpal <[email protected]>
pybamm-team · Jan 25, 2024 · 00916f3 · 00916f3
1 parent 3253bd3
commit 00916f3
Show file tree

Hide file tree

Showing 10 changed files with 44 additions and 154 deletions.
diff --git a/pybamm/__init__.py b/pybamm/__init__.py
@@ -164,7 +164,6 @@
 from .parameters.ecm_parameters import EcmParameters
 from .parameters.size_distribution_parameters import *
 from .parameters.parameter_sets import parameter_sets
-from .parameters_cli import add_parameter, remove_parameter, edit_parameter
 
 #
 # Mesh and Discretisation classes

diff --git a/pybamm/experiment/experiment.py b/pybamm/experiment/experiment.py
@@ -1,7 +1,3 @@
-#
-# Experiment class
-#
-
 from __future__ import annotations
 import pybamm
 from .step._steps_util import (
@@ -47,20 +43,7 @@ def __init__(
         period: str = "1 minute",
         temperature: float | None = None,
         termination: list[str] | None = None,
-        drive_cycles=None,
-        cccv_handling=None,
     ):
-        if cccv_handling is not None:
-            raise ValueError(
-                "cccv_handling has been deprecated, use "
-                "`pybamm.step.cccv_ode(current, voltage)` instead to produce the "
-                "same behavior as the old `cccv_handling='ode'`"
-            )
-        if drive_cycles is not None:
-            raise ValueError(
-                "drive_cycles should now be passed as an experiment step object, e.g. "
-                "`pybamm.step.current(drive_cycle)`"
-            )
         # Save arguments for copying
         self.args = (
             operating_conditions,
@@ -71,7 +54,6 @@ def __init__(
 
         operating_conditions_cycles = []
         for cycle in operating_conditions:
-            # Check types and convert to list
             if not isinstance(cycle, tuple):
                 cycle = (cycle,)
             operating_conditions_cycles.append(cycle)
@@ -84,26 +66,14 @@ def __init__(
         )
 
         # Convert strings to pybamm.step._Step objects
-        # We only do this once per unique step, do avoid unnecessary conversions
+        # We only do this once per unique step, to avoid unnecessary conversions
         # Assign experiment period and temperature if not specified in step
         self.period = _convert_time_to_seconds(period)
         self.temperature = _convert_temperature_to_kelvin(temperature)
 
-        processed_steps = {}
-        for step in self.operating_conditions_steps_unprocessed:
-            if repr(step) in processed_steps:
-                continue
-            elif isinstance(step, str):
-                processed_step = pybamm.step.string(step)
-            elif isinstance(step, pybamm.step._Step):
-                processed_step = step
-
-            if processed_step.period is None:
-                processed_step.period = self.period
-            if processed_step.temperature is None:
-                processed_step.temperature = self.temperature
-
-            processed_steps[repr(step)] = processed_step
+        processed_steps = self.process_steps(
+            self.operating_conditions_steps_unprocessed, self.period, self.temperature
+        )
 
         self.operating_conditions_steps = [
             processed_steps[repr(step)]
@@ -129,6 +99,27 @@ def __init__(
         self.termination_string = termination
         self.termination = self.read_termination(termination)
 
+    @staticmethod
+    def process_steps(unprocessed_steps, period, temp):
+        processed_steps = {}
+        for step in unprocessed_steps:
+            if repr(step) in processed_steps:
+                continue
+            elif isinstance(step, str):
+                processed_step = pybamm.step.string(step)
+            elif isinstance(step, pybamm.step._Step):
+                processed_step = step
+            else:
+                raise TypeError("Operating conditions must be a Step object or string.")
+
+            if processed_step.period is None:
+                processed_step.period = period
+            if processed_step.temperature is None:
+                processed_step.temperature = temp
+
+            processed_steps[repr(step)] = processed_step
+        return processed_steps
+
     def __str__(self):
         return str(self.operating_conditions_cycles)
 
@@ -138,7 +129,8 @@ def copy(self):
     def __repr__(self):
         return f"pybamm.Experiment({self!s})"
 
-    def read_termination(self, termination):
+    @staticmethod
+    def read_termination(termination):
         """
         Read the termination reason. If this condition is hit, the experiment will stop.
         """
@@ -197,7 +189,8 @@ def search_tag(self, tag):
 
         return cycles
 
-    def _set_next_start_time(self, operating_conditions):
+    @staticmethod
+    def _set_next_start_time(operating_conditions):
         if all(isinstance(i, str) for i in operating_conditions):
             return operating_conditions
 
@@ -207,7 +200,7 @@ def _set_next_start_time(self, operating_conditions):
         for op in reversed(operating_conditions):
             if isinstance(op, str):
                 op = pybamm.step.string(op)
-            elif not isinstance(op, pybamm.step._Step):
+            if not isinstance(op, pybamm.step._Step):
                 raise TypeError(
                     "Operating conditions should be strings or _Step objects"
                 )

diff --git a/pybamm/expression_tree/interpolant.py b/pybamm/expression_tree/interpolant.py
@@ -3,7 +3,6 @@
 #
 import numpy as np
 from scipy import interpolate
-import warnings
 
 import pybamm
 
@@ -49,14 +48,6 @@ def __init__(
         extrapolate=True,
         entries_string=None,
     ):
-        # "cubic spline" has been renamed to "cubic"
-        if interpolator == "cubic spline":
-            interpolator = "cubic"
-            warnings.warn(
-                "The 'cubic spline' interpolator has been renamed to 'cubic'.",
-                DeprecationWarning,
-            )
-
         # Check interpolator is valid
         if interpolator not in ["linear", "cubic", "pchip"]:
             raise ValueError(f"interpolator '{interpolator}' not recognised")

diff --git a/pybamm/models/full_battery_models/lithium_ion/electrode_soh.py b/pybamm/models/full_battery_models/lithium_ion/electrode_soh.py
@@ -363,35 +363,6 @@ def __get_electrode_soh_sims_split(self):
         return [x100_sim, x0_sim]
 
     def solve(self, inputs):
-        if "n_Li" in inputs:
-            warnings.warn(
-                "Input 'n_Li' has been replaced by 'Q_Li', which is 'n_Li * F / 3600'. "
-                "This will be automatically calculated for now. "
-                "Q_Li can be read from parameters as 'param.Q_Li_particles_init'",
-                DeprecationWarning,
-            )
-            n_Li = inputs.pop("n_Li")
-            inputs["Q_Li"] = n_Li * pybamm.constants.F.value / 3600
-        if "C_n" in inputs:
-            warnings.warn("Input 'C_n' has been renamed to 'Q_n'", DeprecationWarning)
-            inputs["Q_n"] = inputs.pop("C_n")
-        if "C_p" in inputs:
-            warnings.warn("Input 'C_p' has been renamed to 'Q_p'", DeprecationWarning)
-            inputs["Q_p"] = inputs.pop("C_p")
-        if inputs.pop("V_min", None) is not None:
-            warnings.warn(
-                "V_min has been removed from the inputs. "
-                "The 'Open-circuit voltage at 0% SOC [V]' "
-                "parameter is now used automatically.",
-                DeprecationWarning,
-            )
-        if inputs.pop("V_max", None) is not None:
-            warnings.warn(
-                "V_max has been removed from the inputs. "
-                "The 'Open-circuit voltage at 100% SOC [V]' "
-                "parameter is now used automatically.",
-                DeprecationWarning,
-            )
         ics = self._set_up_solve(inputs)
         try:
             sol = self._solve_full(inputs, ics)

diff --git a/pybamm/parameters_cli.py b/pybamm/parameters_cli.py
diff --git a/pyproject.toml b/pyproject.toml
@@ -134,9 +134,6 @@ all = [
 ]
 
 [project.scripts]
-pybamm_edit_parameter = "pybamm.parameters_cli:edit_parameter"
-pybamm_add_parameter = "pybamm.parameters_cli:add_parameter"
-pybamm_rm_parameter = "pybamm.parameters_cli:remove_parameter"
 pybamm_install_odes = "pybamm.install_odes:main"
 pybamm_install_jax = "pybamm.util:install_jax"
 

diff --git a/tests/unit/test_experiments/test_experiment.py b/tests/unit/test_experiments/test_experiment.py
@@ -67,6 +67,15 @@ def test_cycle_unpacking(self):
         )
         self.assertEqual(experiment.cycle_lengths, [2, 1, 1])
 
+    def test_invalid_step_type(self):
+        unprocessed = {1.0}
+        period = 1
+        temperature = 300.0
+        with self.assertRaisesRegex(
+            TypeError, "Operating conditions must be a Step object or string."
+        ):
+            pybamm.Experiment.process_steps(unprocessed, period, temperature)
+
     def test_str_repr(self):
         conds = ["Discharge at 1 C for 20 seconds", "Charge at 0.5 W for 10 minutes"]
         experiment = pybamm.Experiment(conds)
@@ -91,38 +100,32 @@ def test_bad_strings(self):
         ):
             pybamm.Experiment([(1, 2, 3)])
 
-    def test_deprecations(self):
-        with self.assertRaisesRegex(ValueError, "cccv_handling"):
-            pybamm.Experiment([], cccv_handling="something")
-        with self.assertRaisesRegex(ValueError, "drive_cycles"):
-            pybamm.Experiment([], drive_cycles="something")
-
     def test_termination(self):
         experiment = pybamm.Experiment(["Discharge at 1 C for 20 seconds"])
         self.assertEqual(experiment.termination, {})
 
         experiment = pybamm.Experiment(
-            ["Discharge at 1 C for 20 seconds"], termination="80.7% capacity"
+            ["Discharge at 1 C for 20 seconds"], termination=["80.7% capacity"]
         )
         self.assertEqual(experiment.termination, {"capacity": (80.7, "%")})
 
         experiment = pybamm.Experiment(
-            ["Discharge at 1 C for 20 seconds"], termination="80.7 % capacity"
+            ["Discharge at 1 C for 20 seconds"], termination=["80.7 % capacity"]
         )
         self.assertEqual(experiment.termination, {"capacity": (80.7, "%")})
 
         experiment = pybamm.Experiment(
-            ["Discharge at 1 C for 20 seconds"], termination="4.1Ah capacity"
+            ["Discharge at 1 C for 20 seconds"], termination=["4.1Ah capacity"]
         )
         self.assertEqual(experiment.termination, {"capacity": (4.1, "Ah")})
 
         experiment = pybamm.Experiment(
-            ["Discharge at 1 C for 20 seconds"], termination="4.1 A.h capacity"
+            ["Discharge at 1 C for 20 seconds"], termination=["4.1 A.h capacity"]
         )
         self.assertEqual(experiment.termination, {"capacity": (4.1, "Ah")})
 
         experiment = pybamm.Experiment(
-            ["Discharge at 1 C for 20 seconds"], termination="3V"
+            ["Discharge at 1 C for 20 seconds"], termination=["3V"]
         )
         self.assertEqual(experiment.termination, {"voltage": (3, "V")})
 

diff --git a/tests/unit/test_expression_tree/test_interpolant.py b/tests/unit/test_expression_tree/test_interpolant.py
@@ -45,15 +45,6 @@ def test_errors(self):
                 (np.ones(10), np.ones(12)), np.ones((10, 12)), pybamm.Symbol("a")
             )
 
-    def test_warnings(self):
-        with self.assertWarnsRegex(Warning, "cubic spline"):
-            pybamm.Interpolant(
-                np.linspace(0, 1, 10),
-                np.ones(10),
-                pybamm.Symbol("a"),
-                interpolator="cubic spline",
-            )
-
     def test_interpolation(self):
         x = np.linspace(0, 1, 200)
         y = pybamm.StateVector(slice(0, 2))

diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_electrode_soh.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_electrode_soh.py
@@ -43,14 +43,6 @@ def test_known_solution(self):
                 energy = esoh_solver.theoretical_energy_integral(inputs)
                 self.assertAlmostEqual(sol[key], energy, places=5)
 
-        # should still work with old inputs
-        n_Li = parameter_values.evaluate(param.n_Li_particles_init)
-        inputs = {"V_min": 3, "V_max": 4.2, "n_Li": n_Li, "C_n": Q_n, "C_p": Q_p}
-
-        # Solve the model and check outputs
-        sol = esoh_solver.solve(inputs)
-        self.assertAlmostEqual(sol["Q_Li"], Q_Li, places=5)
-
     def test_known_solution_cell_capacity(self):
         param = pybamm.LithiumIonParameters()
         parameter_values = pybamm.ParameterValues("Mohtat2020")

diff --git a/tests/unit/test_parameters/test_parameters_cli.py b/tests/unit/test_parameters/test_parameters_cli.py