From fabebd669c4acc5f279c28c0e544e178b7e27d3e Mon Sep 17 00:00:00 2001 From: Parth Tripathi Date: Fri, 1 Apr 2022 13:47:49 +0530 Subject: [PATCH 1/5] Add doctests for DFN model --- pybamm/models/full_battery_models/lithium_ion/dfn.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pybamm/models/full_battery_models/lithium_ion/dfn.py b/pybamm/models/full_battery_models/lithium_ion/dfn.py index 7823e33e01..17e72b767a 100644 --- a/pybamm/models/full_battery_models/lithium_ion/dfn.py +++ b/pybamm/models/full_battery_models/lithium_ion/dfn.py @@ -19,6 +19,13 @@ class DFN(BaseModel): option to False allows users to change any number of the submodels before building the complete model (submodels cannot be changed after the model is built). + + Examples + -------- + >>> import pybamm + >>> model = pybamm.lithium_ion.DFN() + >>> model.name + 'Doyle-Fuller-Newman model' References ---------- From 9aa183b02ba92a37c515b8a3af32b8011db6645b Mon Sep 17 00:00:00 2001 From: Parth Tripathi <86532127+parthxtripathi@users.noreply.github.com> Date: Mon, 4 Apr 2022 14:18:31 +0530 Subject: [PATCH 2/5] Update pybamm/models/full_battery_models/lithium_ion/dfn.py Co-authored-by: Saransh --- pybamm/models/full_battery_models/lithium_ion/dfn.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pybamm/models/full_battery_models/lithium_ion/dfn.py b/pybamm/models/full_battery_models/lithium_ion/dfn.py index 17e72b767a..b9ffd60cbd 100644 --- a/pybamm/models/full_battery_models/lithium_ion/dfn.py +++ b/pybamm/models/full_battery_models/lithium_ion/dfn.py @@ -19,7 +19,6 @@ class DFN(BaseModel): option to False allows users to change any number of the submodels before building the complete model (submodels cannot be changed after the model is built). - Examples -------- >>> import pybamm From 75189e6f313083b45fd1de47b81584fd884bd600 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 19 Apr 2022 14:16:30 +0100 Subject: [PATCH 3/5] #2021 remove deprecation errors --- pybamm/expression_tree/symbol.py | 4 -- pybamm/parameters/parameter_values.py | 50 +------------------ pybamm/simulation.py | 9 +--- pybamm/solvers/base_solver.py | 6 --- pybamm/solvers/idaklu_solver.py | 2 - pybamm/solvers/scikits_dae_solver.py | 3 +- pybamm/solvers/scikits_ode_solver.py | 6 --- pybamm/util.py | 16 ------ .../unit/test_expression_tree/test_symbol.py | 8 --- .../test_parameters/test_parameter_values.py | 28 ----------- tests/unit/test_simulation.py | 4 -- tests/unit/test_solvers/test_base_solver.py | 4 -- .../unit/test_solvers/test_scikits_solvers.py | 5 -- tests/unit/test_util.py | 26 ---------- 14 files changed, 4 insertions(+), 167 deletions(-) diff --git a/pybamm/expression_tree/symbol.py b/pybamm/expression_tree/symbol.py index 3753a639a0..61deb464f8 100644 --- a/pybamm/expression_tree/symbol.py +++ b/pybamm/expression_tree/symbol.py @@ -878,10 +878,6 @@ def has_symbol_of_classes(self, symbol_classes): """ return any(isinstance(symbol, symbol_classes) for symbol in self.pre_order()) - def simplify(self, simplified_symbols=None, clear_domains=True): - """`simplify()` has now been removed.""" - raise pybamm.ModelError("simplify is deprecated as it now has no effect") - def to_casadi(self, t=None, y=None, y_dot=None, inputs=None, casadi_symbols=None): """ Convert the expression tree to a CasADi expression tree. diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py index 399df98637..4d66257231 100644 --- a/pybamm/parameters/parameter_values.py +++ b/pybamm/parameters/parameter_values.py @@ -177,18 +177,6 @@ def update_from_chemistry(self, chemistry): if "lithium plating" in chemistry: component_groups += ["lithium plating"] - if "anode" in chemistry.keys(): - raise KeyError( - "The 'anode' notation has been deprecated, " - "'negative electrode' should be used instead." - ) - - if "cathode" in chemistry.keys(): - raise KeyError( - "The 'cathode' notation has been deprecated, " - "'positive electrode' should be used instead." - ) - for component_group in component_groups: # Make sure component is provided try: @@ -352,43 +340,9 @@ def check_parameter_values(self, values): "'Typical current [A]' cannot be zero. A possible alternative is to " "set 'Current function [A]' to `0` instead." ) - if "C-rate" in values: - raise ValueError( - "The 'C-rate' parameter has been deprecated, " - "use 'Current function [A]' instead. The Nominal " - "cell capacity can be accessed as 'Nominal cell " - "capacity [A.h]', and used to calculate current from C-rate." - ) - if "Cell capacity [A.h]" in values: - raise ValueError( - "The 'Cell capacity [A.h]' parameter has been deprecated, " - "'Nominal cell capacity [A.h]' should be used instead." - ) + for param in values: - if "surface area density" in param: - raise ValueError( - "Parameters involving 'surface area density' have been renamed to " - "'surface area to volume ratio' ('{}' found)".format(param) - ) - elif "reaction rate" in param: - raise ValueError( - "Parameters involving 'reaction rate' have been replaced with " - "'exchange-current density' ('{}' found)".format(param) - ) - elif "particle distribution in x" in param: - raise ValueError( - "The parameter '{}' has been deprecated".format(param) - + "The particle radius is now set as a function of x directly " - "instead of providing a reference value and a distribution." - ) - elif "surface area to volume ratio distribution in x" in param: - raise ValueError( - "The parameter '{}' has been deprecated".format(param) - + "The surface area to volume ratio is now set as a function " - "of x directly instead of providing a reference value and a " - "distribution." - ) - elif "propotional term" in param: + if "propotional term" in param: raise ValueError( f"The parameter '{param}' has been renamed to " "'... proportional term [s-1]', and its value should now be divided" diff --git a/pybamm/simulation.py b/pybamm/simulation.py index 3b89a340cb..693dc7b735 100644 --- a/pybamm/simulation.py +++ b/pybamm/simulation.py @@ -963,7 +963,7 @@ def step( return self.solution - def plot(self, output_variables=None, quick_plot_vars=None, **kwargs): + def plot(self, output_variables=None, **kwargs): """ A method to quickly plot the outputs of the simulation. Creates a :class:`pybamm.QuickPlot` object (with keyword arguments 'kwargs') and @@ -973,19 +973,12 @@ def plot(self, output_variables=None, quick_plot_vars=None, **kwargs): ---------- output_variables: list, optional A list of the variables to plot. - quick_plot_vars: list, optional - A list of the variables to plot. Deprecated, use output_variables instead. **kwargs Additional keyword arguments passed to :meth:`pybamm.QuickPlot.dynamic_plot`. For a list of all possible keyword arguments see :class:`pybamm.QuickPlot`. """ - if quick_plot_vars is not None: - raise NotImplementedError( - "'quick_plot_vars' has been deprecated. Use 'output_variables' instead." - ) - if self._solution is None: raise ValueError( "Model has not been solved, please solve the model before plotting." diff --git a/pybamm/solvers/base_solver.py b/pybamm/solvers/base_solver.py index a8f9d9cb72..0888ff7f95 100644 --- a/pybamm/solvers/base_solver.py +++ b/pybamm/solvers/base_solver.py @@ -48,7 +48,6 @@ def __init__( root_method=None, root_tol=1e-6, extrap_tol=0, - max_steps="deprecated", ): self.method = method self.rtol = rtol @@ -56,11 +55,6 @@ def __init__( self.root_tol = root_tol self.root_method = root_method self.extrap_tol = extrap_tol - if max_steps != "deprecated": - raise ValueError( - "max_steps has been deprecated, and should be set using the " - "solver-specific extra-options dictionaries instead" - ) self.models_set_up = {} # Defaults, can be overwritten by specific solver diff --git a/pybamm/solvers/idaklu_solver.py b/pybamm/solvers/idaklu_solver.py index 3a0753deb5..8f6a8f1909 100644 --- a/pybamm/solvers/idaklu_solver.py +++ b/pybamm/solvers/idaklu_solver.py @@ -50,7 +50,6 @@ def __init__( root_method="casadi", root_tol=1e-6, extrap_tol=0, - max_steps="deprecated", ): if idaklu_spec is None: # pragma: no cover @@ -63,7 +62,6 @@ def __init__( root_method, root_tol, extrap_tol, - max_steps, ) self.name = "IDA KLU solver" diff --git a/pybamm/solvers/scikits_dae_solver.py b/pybamm/solvers/scikits_dae_solver.py index cdc86d81ef..1d0550b81e 100644 --- a/pybamm/solvers/scikits_dae_solver.py +++ b/pybamm/solvers/scikits_dae_solver.py @@ -56,13 +56,12 @@ def __init__( root_tol=1e-6, extrap_tol=0, extra_options=None, - max_steps="deprecated", ): if scikits_odes_spec is None: raise ImportError("scikits.odes is not installed") super().__init__( - method, rtol, atol, root_method, root_tol, extrap_tol, max_steps + method, rtol, atol, root_method, root_tol, extrap_tol ) self.name = "Scikits DAE solver ({})".format(method) diff --git a/pybamm/solvers/scikits_ode_solver.py b/pybamm/solvers/scikits_ode_solver.py index addeb728f5..186b8f04eb 100644 --- a/pybamm/solvers/scikits_ode_solver.py +++ b/pybamm/solvers/scikits_ode_solver.py @@ -49,7 +49,6 @@ def __init__( rtol=1e-6, atol=1e-6, extrap_tol=0, - linsolver="deprecated", extra_options=None, ): if scikits_odes_spec is None: @@ -57,11 +56,6 @@ def __init__( super().__init__(method, rtol, atol, extrap_tol=extrap_tol) self.extra_options = extra_options or {} - if linsolver != "deprecated": - raise ValueError( - "linsolver has been deprecated. Pass 'linsolver' to extra_options " - "dictionary instead" - ) self.ode_solver = True self.name = "Scikits ODE solver ({})".format(method) diff --git a/pybamm/util.py b/pybamm/util.py index 405977f0a5..52ef324c5c 100644 --- a/pybamm/util.py +++ b/pybamm/util.py @@ -265,22 +265,6 @@ def load_function(filename): if filename.endswith(".py"): filename = filename.replace(".py", "") - # Replace `lead-acid` with `lead_acid` - if "lead-acid" in filename: - warnings.simplefilter("always", DeprecationWarning) - warnings.warn( - "lead-acid is deprecated, use lead_acid instead", DeprecationWarning - ) - filename = filename.replace("lead-acid", "lead_acid") - - # Replace `lithium-ion` with `lithium_ion` - if "lithium-ion" in filename: - warnings.simplefilter("always", DeprecationWarning) - warnings.warn( - "lithium-ion is deprecated, use lithium_ion instead", DeprecationWarning - ) - filename = filename.replace("lithium-ion", "lithium_ion") - # Assign path to _ and filename to tail _, tail = os.path.split(filename) diff --git a/tests/unit/test_expression_tree/test_symbol.py b/tests/unit/test_expression_tree/test_symbol.py index 6adcb57433..4b972eab49 100644 --- a/tests/unit/test_expression_tree/test_symbol.py +++ b/tests/unit/test_expression_tree/test_symbol.py @@ -319,14 +319,6 @@ def test_symbol_evaluates_to_constant_number(self): a = 3 * pybamm.t + 2 self.assertFalse(a.evaluates_to_constant_number()) - def test_simplify(self): - a = pybamm.Parameter("A") - # test error - with self.assertRaisesRegex( - pybamm.ModelError, "simplify is deprecated as it now has no effect" - ): - (a + a).simplify() - def test_simplify_if_constant(self): m = pybamm.Matrix(np.zeros((10, 10))) m_simp = pybamm.simplify_if_constant(m) diff --git a/tests/unit/test_parameters/test_parameter_values.py b/tests/unit/test_parameters/test_parameter_values.py index f57003019e..54554cf77f 100644 --- a/tests/unit/test_parameters/test_parameter_values.py +++ b/tests/unit/test_parameters/test_parameter_values.py @@ -137,26 +137,9 @@ def test_update(self): param.update({"b": 1}) def test_check_parameter_values(self): - # Cell capacity [A.h] deprecated - with self.assertRaisesRegex(ValueError, "Cell capacity"): - pybamm.ParameterValues({"Cell capacity [A.h]": 1}) # Can't provide a current density of 0, as this will cause a ZeroDivision error with self.assertRaisesRegex(ValueError, "Typical current"): pybamm.ParameterValues({"Typical current [A]": 0}) - with self.assertRaisesRegex( - ValueError, "The 'C-rate' parameter has been deprecated" - ): - pybamm.ParameterValues({"C-rate": 0}) - with self.assertRaisesRegex(ValueError, "surface area density"): - pybamm.ParameterValues({"Negative surface area density": 1}) - with self.assertRaisesRegex(ValueError, "reaction rate"): - pybamm.ParameterValues({"Negative reaction rate": 1}) - with self.assertRaisesRegex(ValueError, "particle distribution"): - pybamm.ParameterValues({"Negative particle distribution in x": 1}) - with self.assertRaisesRegex(ValueError, "surface area to volume ratio"): - pybamm.ParameterValues( - {"Negative electrode surface area to volume ratio distribution in x": 1} - ) with self.assertRaisesRegex(ValueError, "propotional term"): pybamm.ParameterValues( {"Negative electrode LAM constant propotional term": 1} @@ -1002,17 +985,6 @@ def some_function(self): self.assertEqual(df[1]["b"], "[function]some_function") self.assertEqual(df[1]["c"], "[data]some_data") - def test_deprecate_anode_cathode(self): - chemistry = pybamm.parameter_sets.Ecker2015.copy() - chemistry["anode"] = chemistry.pop("negative electrode") - with self.assertRaisesRegex(KeyError, "anode"): - pybamm.ParameterValues(chemistry) - - chemistry = pybamm.parameter_sets.Ecker2015.copy() - chemistry["cathode"] = chemistry.pop("positive electrode") - with self.assertRaisesRegex(KeyError, "cathode"): - pybamm.ParameterValues(chemistry) - if __name__ == "__main__": print("Add -v for more debug output") diff --git a/tests/unit/test_simulation.py b/tests/unit/test_simulation.py index c0e23a66d7..b0c6f5840e 100644 --- a/tests/unit/test_simulation.py +++ b/tests/unit/test_simulation.py @@ -362,10 +362,6 @@ def test_plot(self): sim.solve(t_eval=t_eval) sim.plot(testing=True) - # test quick_plot_vars deprecation error - with self.assertRaisesRegex(NotImplementedError, "'quick_plot_vars'"): - sim.plot(quick_plot_vars=["var"]) - def test_create_gif(self): sim = pybamm.Simulation(pybamm.lithium_ion.SPM()) sim.solve(t_eval=[0, 10]) diff --git a/tests/unit/test_solvers/test_base_solver.py b/tests/unit/test_solvers/test_base_solver.py index a96adab593..5592974e46 100644 --- a/tests/unit/test_solvers/test_base_solver.py +++ b/tests/unit/test_solvers/test_base_solver.py @@ -20,10 +20,6 @@ def test_base_solver_init(self): solver.rtol = 1e-7 self.assertEqual(solver.rtol, 1e-7) - # max_steps deprecated - with self.assertRaisesRegex(ValueError, "max_steps has been deprecated"): - pybamm.BaseSolver(max_steps=10) - def test_root_method_init(self): solver = pybamm.BaseSolver(root_method="casadi") self.assertIsInstance(solver.root_method, pybamm.CasadiAlgebraicSolver) diff --git a/tests/unit/test_solvers/test_scikits_solvers.py b/tests/unit/test_solvers/test_scikits_solvers.py index d6f6e561da..7d3fef0c4d 100644 --- a/tests/unit/test_solvers/test_scikits_solvers.py +++ b/tests/unit/test_solvers/test_scikits_solvers.py @@ -11,11 +11,6 @@ @unittest.skipIf(not pybamm.have_scikits_odes(), "scikits.odes not installed") class TestScikitsSolvers(unittest.TestCase): - def test_init(self): - # linsolver deprecated - with self.assertRaisesRegex(ValueError, "linsolver has been deprecated"): - pybamm.ScikitsOdeSolver(linsolver="lapackdense") - def test_model_ode_integrate_failure(self): # Turn off warnings to ignore sqrt error warnings.simplefilter("ignore") diff --git a/tests/unit/test_util.py b/tests/unit/test_util.py index 06781ee815..0b64fecc1a 100644 --- a/tests/unit/test_util.py +++ b/tests/unit/test_util.py @@ -25,32 +25,6 @@ class TestUtil(unittest.TestCase): """ def test_load_function(self): - # Test replace function and deprecation warning for lithium-ion - with self.assertWarns(Warning): - warn_path = os.path.join( - "pybamm", - "input", - "parameters", - "lithium-ion", - "negative_electrodes", - "graphite_Chen2020", - "graphite_LGM50_electrolyte_exchange_current_density_Chen2020.py", - ) - pybamm.load_function(warn_path) - - # Test replace function and deprecation warning for lead-acid - with self.assertWarns(Warning): - warn_path = os.path.join( - "pybamm", - "input", - "parameters", - "lead-acid", - "negative_electrodes", - "lead_Sulzer2019", - "lead_exchange_current_density_Sulzer2019.py", - ) - pybamm.load_function(warn_path) - # Test function load with absolute path abs_test_path = os.path.join( pybamm.root_dir(), From e59fe35c51baf1414ac26d1b9fa5ce9996c88046 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 19 Apr 2022 14:27:35 +0100 Subject: [PATCH 4/5] flake8 --- pybamm/util.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pybamm/util.py b/pybamm/util.py index 52ef324c5c..1356a7fe18 100644 --- a/pybamm/util.py +++ b/pybamm/util.py @@ -13,7 +13,6 @@ import subprocess import sys import timeit -import warnings from collections import defaultdict from platform import system From 00b8b0c8c7c48d39ea2432e75334988ff495a3ee Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Tue, 19 Apr 2022 15:34:03 +0100 Subject: [PATCH 5/5] #2021 update CHANGELOG --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70b636b1a7..75d33337dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # [Unreleased](https://github.com/pybamm-team/PyBaMM/) +## Features + +## Bug fixes + +- Remove old deprecation errors, including those in `parameter_values.py` that caused the simulation if, for example, the reaction rate is re-introduced manually ([#2022](https://github.com/pybamm-team/PyBaMM/pull/2022)) + +## Breaking changes + # [v22.3](https://github.com/pybamm-team/PyBaMM/tree/v22.3) - 2022-03-31 ## Features