From 092ebc98dd42c123289b431fa18d897c999b692d Mon Sep 17 00:00:00 2001 From: Brady Planden <55357039+BradyPlanden@users.noreply.github.com> Date: Wed, 22 May 2024 14:56:03 +0100 Subject: [PATCH] Fixes breaking changes from #3624 (#4072) * removes breaking change from electrode diffusivity -> particle diffusivity * Add conditional entry for base_solver inputs recreation * fix: incorrect depreciated parameter translation, moves conversion to ParameterValues staticmethod Fixes an incorrect conversion of electrode diffusivity to particle diffusivity, Updates the corresponding test to ensure FuzzyDict.__getitem__ functions correctly, Adds conversion and checks to BaseSolver inputs object, * Add changelog entry --------- Co-authored-by: Eric G. Kratz --- CHANGELOG.md | 1 + pybamm/parameters/parameter_values.py | 15 +++++++++++++-- pybamm/solvers/base_solver.py | 6 +++++- pybamm/util.py | 6 +++--- tests/unit/test_util.py | 7 ++----- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb02ebedb7..b72151f36c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ ## Bug Fixes +- Fixes the breaking changes caused by [#3624](https://github.com/pybamm-team/PyBaMM/pull/3624), specifically enables the deprecated parameter `electrode diffusivity` to be used by `ParameterValues.update({name:value})` and `Solver.solve(inputs={name:value})`. Fixes parameter translation from old name to new name, with corrected tests. ([#4072](https://github.com/pybamm-team/PyBaMM/pull/4072) - Set the `remove_independent_variables_from_rhs` to `False` by default, and moved the option from `Discretisation.process_model` to `Discretisation.__init__`. This fixes a bug related to the discharge capacity, but may make the simulation slower in some cases. To set the option to `True`, use `Simulation(..., discretisation_kwargs={"remove_independent_variables_from_rhs": True})`. ([#4020](https://github.com/pybamm-team/PyBaMM/pull/4020)) - Fixed a bug where independent variables were removed from models even if they appeared in events ([#4019](https://github.com/pybamm-team/PyBaMM/pull/4019)) - Fix bug with upwind and downwind schemes producing the wrong discretised system ([#3979](https://github.com/pybamm-team/PyBaMM/pull/3979)) diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py index 402000b5a1..8301771044 100644 --- a/pybamm/parameters/parameter_values.py +++ b/pybamm/parameters/parameter_values.py @@ -5,6 +5,7 @@ import pybamm import numbers from pprint import pformat +from warnings import warn from collections import defaultdict @@ -229,7 +230,7 @@ def update(self, values, check_conflict=False, check_already_exists=True, path=" if not isinstance(values, dict): values = values._dict_items # check parameter values - self.check_parameter_values(values) + values = self.check_parameter_values(values) # update for name, value in values.items(): # check for conflicts @@ -389,7 +390,8 @@ def set_initial_ocps( ) return parameter_values - def check_parameter_values(self, values): + @staticmethod + def check_parameter_values(values): for param in values: if "propotional term" in param: raise ValueError( @@ -402,6 +404,15 @@ def check_parameter_values(self, values): raise ValueError( f"parameter '{param}' has been renamed to " "'Thermodynamic factor'" ) + if "electrode diffusivity" in param: + warn( + f"The parameter '{param}' has been renamed to '{param.replace('electrode', 'particle')}'", + DeprecationWarning, + stacklevel=2, + ) + param = param.replace("electrode", "particle") + + return values def process_model(self, unprocessed_model, inplace=True): """Assign parameter values to a model. diff --git a/pybamm/solvers/base_solver.py b/pybamm/solvers/base_solver.py index 625de7a3ab..798ff9f19a 100644 --- a/pybamm/solvers/base_solver.py +++ b/pybamm/solvers/base_solver.py @@ -12,6 +12,7 @@ import pybamm from pybamm.expression_tree.binary_operators import _Heaviside +from pybamm import ParameterValues class BaseSolver: @@ -1401,7 +1402,10 @@ def get_platform_context(self, system_type: str): @staticmethod def _set_up_model_inputs(model, inputs): """Set up input parameters""" - inputs = inputs or {} + if inputs is None: + inputs = {} + else: + inputs = ParameterValues.check_parameter_values(inputs) # Go through all input parameters that can be found in the model # Only keep the ones that are actually used in the model diff --git a/pybamm/util.py b/pybamm/util.py index ac7e5f6475..e5ec0d8125 100644 --- a/pybamm/util.py +++ b/pybamm/util.py @@ -57,14 +57,14 @@ def __getitem__(self, key): try: return super().__getitem__(key) except KeyError as error: - if "particle diffusivity" in key: + if "electrode diffusivity" in key: warn( - f"The parameter '{key.replace('particle', 'electrode')}' " + f"The parameter '{key.replace('electrode', 'particle')}' " f"has been renamed to '{key}'", DeprecationWarning, stacklevel=2, ) - return super().__getitem__(key.replace("particle", "electrode")) + return super().__getitem__(key.replace("electrode", "particle")) if key in ["Negative electrode SOC", "Positive electrode SOC"]: domain = key.split(" ")[0] raise KeyError( diff --git a/tests/unit/test_util.py b/tests/unit/test_util.py index 603af50560..2daad1f6a3 100644 --- a/tests/unit/test_util.py +++ b/tests/unit/test_util.py @@ -36,7 +36,7 @@ def test_fuzzy_dict(self): "SEI current": 3, "Lithium plating current": 4, "A dimensional variable [m]": 5, - "Positive electrode diffusivity [m2.s-1]": 6, + "Positive particle diffusivity [m2.s-1]": 6, } ) self.assertEqual(d["test"], 1) @@ -59,10 +59,7 @@ def test_fuzzy_dict(self): d.__getitem__("Open-circuit voltage at 100% SOC [V]") with self.assertWarns(DeprecationWarning): - self.assertEqual( - d["Positive electrode diffusivity [m2.s-1]"], - d["Positive particle diffusivity [m2.s-1]"], - ) + self.assertEqual(d["Positive electrode diffusivity [m2.s-1]"], 6) def test_get_parameters_filepath(self): tempfile_obj = tempfile.NamedTemporaryFile("w", dir=".")