Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2021 remove deprecation errors #2022

Merged
merged 3 commits into from
Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 0 additions & 4 deletions pybamm/expression_tree/symbol.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
50 changes: 2 additions & 48 deletions pybamm/parameters/parameter_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"
Expand Down
9 changes: 1 addition & 8 deletions pybamm/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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."
Expand Down
6 changes: 0 additions & 6 deletions pybamm/solvers/base_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,13 @@ def __init__(
root_method=None,
root_tol=1e-6,
extrap_tol=0,
max_steps="deprecated",
):
self.method = method
self.rtol = rtol
self.atol = atol
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
Expand Down
2 changes: 0 additions & 2 deletions pybamm/solvers/idaklu_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -63,7 +62,6 @@ def __init__(
root_method,
root_tol,
extrap_tol,
max_steps,
)
self.name = "IDA KLU solver"

Expand Down
3 changes: 1 addition & 2 deletions pybamm/solvers/scikits_dae_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
6 changes: 0 additions & 6 deletions pybamm/solvers/scikits_ode_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,13 @@ def __init__(
rtol=1e-6,
atol=1e-6,
extrap_tol=0,
linsolver="deprecated",
extra_options=None,
):
if scikits_odes_spec is None:
raise ImportError("scikits.odes is not installed")

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)

Expand Down
17 changes: 0 additions & 17 deletions pybamm/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import subprocess
import sys
import timeit
import warnings
from collections import defaultdict
from platform import system

Expand Down Expand Up @@ -265,22 +264,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)

Expand Down
8 changes: 0 additions & 8 deletions tests/unit/test_expression_tree/test_symbol.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 0 additions & 28 deletions tests/unit/test_parameters/test_parameter_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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")
Expand Down
4 changes: 0 additions & 4 deletions tests/unit/test_simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
4 changes: 0 additions & 4 deletions tests/unit/test_solvers/test_base_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions tests/unit/test_solvers/test_scikits_solvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
26 changes: 0 additions & 26 deletions tests/unit/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down