Skip to content

Commit

Permalink
Fixes breaking changes from #3624 (#4072)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
BradyPlanden and kratman authored May 22, 2024
1 parent 5f35628 commit 092ebc9
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
15 changes: 13 additions & 2 deletions pybamm/parameters/parameter_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import pybamm
import numbers
from pprint import pformat
from warnings import warn
from collections import defaultdict


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion pybamm/solvers/base_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import pybamm
from pybamm.expression_tree.binary_operators import _Heaviside
from pybamm import ParameterValues


class BaseSolver:
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pybamm/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 2 additions & 5 deletions tests/unit/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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=".")
Expand Down

0 comments on commit 092ebc9

Please sign in to comment.