From 77f6361fa4828ec54dad31e2b7ccbcd0fe3c6631 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Thu, 23 Jan 2020 20:51:14 -0500 Subject: [PATCH 1/9] #790 added some fuzzy dictionaries and checks when updating parameter values --- .requirements-docs.txt | 2 + pybamm/__init__.py | 2 +- pybamm/models/base_model.py | 4 +- .../full_battery_models/base_battery_model.py | 2 +- pybamm/parameters/parameter_values.py | 68 ++++++++++++------- pybamm/util.py | 11 +++ setup.py | 2 + .../test_parameters/test_parameter_values.py | 4 ++ tests/unit/test_simulation.py | 4 +- tests/unit/test_util.py | 6 ++ 10 files changed, 77 insertions(+), 28 deletions(-) diff --git a/.requirements-docs.txt b/.requirements-docs.txt index c7bcd6446f..1f9addea77 100644 --- a/.requirements-docs.txt +++ b/.requirements-docs.txt @@ -8,3 +8,5 @@ scikit-fem>=0.2.0 casadi>=3.5.0 guzzle-sphinx-theme sphinx>=1.5 +fuzzywuzzy>=0.1.0 +python-Levenshtein>=0.12.0 diff --git a/pybamm/__init__.py b/pybamm/__init__.py index 265697628c..ea8fbf43e0 100644 --- a/pybamm/__init__.py +++ b/pybamm/__init__.py @@ -55,7 +55,7 @@ def version(formatted=False): # # Utility classes and methods # -from .util import Timer +from .util import Timer, FuzzyDict from .util import root_dir, load_function, rmse, get_infinite_nested_dict, load from .logger import logger, set_logging_level from .settings import settings diff --git a/pybamm/models/base_model.py b/pybamm/models/base_model.py index 360a5510de..073b4e1887 100644 --- a/pybamm/models/base_model.py +++ b/pybamm/models/base_model.py @@ -104,7 +104,7 @@ def __init__(self, name="Unnamed model"): self._algebraic = {} self._initial_conditions = {} self._boundary_conditions = {} - self._variables = {} + self._variables = pybamm.FuzzyDict() self._events = {} self._concatenated_rhs = None self._concatenated_algebraic = None @@ -208,7 +208,7 @@ def variables(self): @variables.setter def variables(self, variables): - self._variables = variables + self._variables.update(variables) def variable_names(self): return list(self._variables.keys()) diff --git a/pybamm/models/full_battery_models/base_battery_model.py b/pybamm/models/full_battery_models/base_battery_model.py index d8c17e2416..08b764e946 100644 --- a/pybamm/models/full_battery_models/base_battery_model.py +++ b/pybamm/models/full_battery_models/base_battery_model.py @@ -341,7 +341,7 @@ def set_standard_output_variables(self): } ) - self.variables = {} + self._variables = pybamm.FuzzyDict() # Time time_scale = pybamm.electrical_parameters.timescale diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py index 4f5ecdc250..c3a4bc4bcb 100644 --- a/pybamm/parameters/parameter_values.py +++ b/pybamm/parameters/parameter_values.py @@ -7,9 +7,12 @@ import numbers -class ParameterValues(dict): +class ParameterValues: """ - The parameter values for a simulation. + The parameter values for a simulation. + + Note that this class does not inherit directly from the python dictionary class as + this causes issues with saving and loading simulations. Parameters ---------- @@ -49,6 +52,7 @@ class ParameterValues(dict): """ def __init__(self, values=None, chemistry=None): + self.items = pybamm.FuzzyDict() # Must provide either values or chemistry, not both (nor neither) if values is not None and chemistry is not None: raise ValueError( @@ -66,19 +70,28 @@ def __init__(self, values=None, chemistry=None): self.update_from_chemistry(chemistry) # Then update with values dictionary or file if values is not None: + # If base_parameters is a filename, load from that filename if isinstance(values, str): values = self.read_parameters_csv(values) - # If base_parameters is a filename, load from that filename - self.update(values) + # Don't check parameter already exists when first creating it + self.update(values, check_already_exists=False) # Initialise empty _processed_symbols dict (for caching) self._processed_symbols = {} def __getitem__(self, key): - try: - return super().__getitem__(key) - except KeyError as err: - raise KeyError("Parameter '{}' not recognised".format(err.args[0])) + return self.items[key] + + def __setitem__(self, key, value): + "Call the update functionality when doing a setitem" + self.update({key: value}) + + def __delitem__(self, key): + del self.items[key] + + def keys(self): + "Get the keys of the dictionary" + return self.items.keys() def update_from_chemistry(self, chemistry): """ @@ -111,7 +124,12 @@ def update_from_chemistry(self, chemistry): os.path.join(component_path, "parameters.csv") ) # Update parameters, making sure to check any conflicts - self.update(component_params, check_conflict=True, path=component_path) + self.update( + component_params, + check_conflict=True, + check_already_exists=False, + path=component_path, + ) def read_parameters_csv(self, filename): """Reads parameters from csv file into dict. @@ -132,11 +150,7 @@ def read_parameters_csv(self, filename): df.dropna(how="all", inplace=True) return {k: v for (k, v) in zip(df["Name [units]"], df["Value"])} - def __setitem__(self, key, value): - "Call the update functionality when doing a setitem" - self.update({key: value}) - - def update(self, values, check_conflict=False, path=""): + def update(self, values, check_conflict=False, check_already_exists=True, path=""): # update for name, value in values.items(): # check for conflicts @@ -150,6 +164,14 @@ def update(self, values, check_conflict=False, path=""): name, self[name] ) ) + # check parameter already exists (for updating parameters) + if check_already_exists is True and name not in self.items.keys(): + raise KeyError( + """cannot update parameter '{}' as it does not have a default value + """.format( + name + ) + ) # if no conflicts, update, loading functions and data if they are specified else: # Functions are flagged with the string "[function]" @@ -158,7 +180,7 @@ def update(self, values, check_conflict=False, path=""): loaded_value = pybamm.load_function( os.path.join(path, value[10:] + ".py") ) - super().__setitem__(name, loaded_value) + self.items[name] = loaded_value values[name] = loaded_value # Data is flagged with the string "[data]" or "[current data]" elif value.startswith("[current data]") or value.startswith( @@ -177,16 +199,16 @@ def update(self, values, check_conflict=False, path=""): filename, comment="#", skip_blank_lines=True ).to_numpy() # Save name and data - super().__setitem__(name, (function_name, data)) + self.items[name] = (function_name, data) values[name] = (function_name, data) elif value == "[input]": - super().__setitem__(name, pybamm.InputParameter(name)) + self.items[name] = pybamm.InputParameter(name) # Anything else should be a converted to a float else: - super().__setitem__(name, float(value)) + self.items[name] = float(value) values[name] = float(value) else: - super().__setitem__(name, value) + self.items[name] = value # check parameter values self.check_and_update_parameter_values(values) # reset processed symbols @@ -209,12 +231,12 @@ def check_and_update_parameter_values(self, values): ) # If the capacity of the cell has been provided, make sure "C-rate" and current # match with the stated capacity - if "Cell capacity [A.h]" in values or "Cell capacity [A.h]" in self: + if "Cell capacity [A.h]" in values or "Cell capacity [A.h]" in self.items: # Capacity from values takes precedence if "Cell capacity [A.h]" in values: capacity = values["Cell capacity [A.h]"] else: - capacity = self["Cell capacity [A.h]"] + capacity = self.items["Cell capacity [A.h]"] # Make sure they match if both provided # Update the other if only one provided if "C-rate" in values: @@ -227,7 +249,7 @@ def check_and_update_parameter_values(self, values): value = (values["C-rate"][0] + "_to_Crate", data) else: value = values["C-rate"] * capacity - super().__setitem__("Current function [A]", value) + self.items["Current function [A]"] = value elif "Current function [A]" in values: if callable(values["Current function [A]"]): value = CurrentToCrate(values["Current function [A]"], capacity) @@ -237,7 +259,7 @@ def check_and_update_parameter_values(self, values): value = (values["Current function [A]"][0] + "_to_current", data) else: value = values["Current function [A]"] / capacity - super().__setitem__("C-rate", value) + self.items["C-rate"] = value return values diff --git a/pybamm/util.py b/pybamm/util.py index c320289b9a..16042bd1f8 100644 --- a/pybamm/util.py +++ b/pybamm/util.py @@ -13,6 +13,7 @@ import pickle import pybamm from collections import defaultdict +from fuzzywuzzy import process def root_dir(): @@ -20,6 +21,16 @@ def root_dir(): return str(pathlib.Path(pybamm.__path__[0]).parent) +class FuzzyDict(dict): + def __getitem__(self, key): + try: + return super().__getitem__(key) + except KeyError: + best_matches = process.extract(key, self.keys(), limit=3) + best_matches = [match[0] for match in best_matches] + raise KeyError(f"'{key}' not found. Best matches are {best_matches}") + + class Timer(object): """ Provides accurate timing. diff --git a/setup.py b/setup.py index fbaebb09cb..29a517b247 100644 --- a/setup.py +++ b/setup.py @@ -48,6 +48,8 @@ def load_version(): "scikit-fem>=0.2.0", "casadi>=3.5.0", "jupyter", # For example notebooks + "fuzzywuzzy>=0.17.0", + "python-Levenshtein>=0.12.0", # Note: Matplotlib is loaded for debug plots, but to ensure pybamm runs # on systems without an attached display, it should never be imported # outside of plot() methods. diff --git a/tests/unit/test_parameters/test_parameter_values.py b/tests/unit/test_parameters/test_parameter_values.py index a41b3e5eaf..c24a1f6a41 100644 --- a/tests/unit/test_parameters/test_parameter_values.py +++ b/tests/unit/test_parameters/test_parameter_values.py @@ -57,6 +57,9 @@ def test_update(self): ValueError, "parameter 'a' already defined with value '3'" ): param.update({"a": 4}, check_conflict=True) + # with parameter not existing yet + with self.assertRaisesRegex(KeyError, "cannot update parameter"): + param.update({"b": 1}) def test_check_and_update_parameter_values(self): # Can't provide a current density of 0, as this will cause a ZeroDivision error @@ -367,6 +370,7 @@ def test_interpolant_against_function(self): "cathodes", "lico2_Marquis2019", ), + check_already_exists=False, ) a = pybamm.Parameter("a") diff --git a/tests/unit/test_simulation.py b/tests/unit/test_simulation.py index fcfaf2831f..4dc0a8f103 100644 --- a/tests/unit/test_simulation.py +++ b/tests/unit/test_simulation.py @@ -294,7 +294,9 @@ def test_set_defaults2(self): sim.set_defaults() # Not sure of best way to test nested dicts? # self.geometry = model.default_geometry - self.assertEqual(sim._parameter_values, model.default_parameter_values) + self.assertEqual( + sim._parameter_values.items, model.default_parameter_values.items + ) for domain, submesh in model.default_submesh_types.items(): self.assertEqual( sim._submesh_types[domain].submesh_type, submesh.submesh_type diff --git a/tests/unit/test_util.py b/tests/unit/test_util.py index 465bfcfd29..1d9eb66c2b 100644 --- a/tests/unit/test_util.py +++ b/tests/unit/test_util.py @@ -77,6 +77,12 @@ def test_infinite_nested_dict(self): d[4][5] = "y" self.assertEqual(d[4][5], "y") + def test_fuzzy_dict(self): + d = pybamm.FuzzyDict({"test": 1, "test2": 2}) + self.assertEqual(d["test"], 1) + with self.assertRaisesRegex(KeyError, "'test3' not found. Best matches are "): + d["test3"] + if __name__ == "__main__": print("Add -v for more debug output") From db6fe131005b2c47bb3bfc14ef6c5ec402343dbf Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Thu, 23 Jan 2020 21:05:44 -0500 Subject: [PATCH 2/9] #790 make parameter values use fuzzy dict error --- pybamm/parameters/parameter_values.py | 78 +++++++++++++-------------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py index c3a4bc4bcb..ac8c3eb4d7 100644 --- a/pybamm/parameters/parameter_values.py +++ b/pybamm/parameters/parameter_values.py @@ -165,50 +165,50 @@ def update(self, values, check_conflict=False, check_already_exists=True, path=" ) ) # check parameter already exists (for updating parameters) - if check_already_exists is True and name not in self.items.keys(): - raise KeyError( - """cannot update parameter '{}' as it does not have a default value - """.format( - name + if check_already_exists is True: + try: + self.items[name] + except KeyError as err: + raise KeyError( + "cannot update parameter '{}' ".format(name) + + "as it does not have a default value. ({})".format( + err.args[0] + ) ) - ) # if no conflicts, update, loading functions and data if they are specified - else: - # Functions are flagged with the string "[function]" - if isinstance(value, str): - if value.startswith("[function]"): - loaded_value = pybamm.load_function( - os.path.join(path, value[10:] + ".py") + # Functions are flagged with the string "[function]" + if isinstance(value, str): + if value.startswith("[function]"): + loaded_value = pybamm.load_function( + os.path.join(path, value[10:] + ".py") + ) + self.items[name] = loaded_value + values[name] = loaded_value + # Data is flagged with the string "[data]" or "[current data]" + elif value.startswith("[current data]") or value.startswith("[data]"): + if value.startswith("[current data]"): + data_path = os.path.join( + pybamm.root_dir(), "input", "drive_cycles" ) - self.items[name] = loaded_value - values[name] = loaded_value - # Data is flagged with the string "[data]" or "[current data]" - elif value.startswith("[current data]") or value.startswith( - "[data]" - ): - if value.startswith("[current data]"): - data_path = os.path.join( - pybamm.root_dir(), "input", "drive_cycles" - ) - filename = os.path.join(data_path, value[14:] + ".csv") - function_name = value[14:] - else: - filename = os.path.join(path, value[6:] + ".csv") - function_name = value[6:] - data = pd.read_csv( - filename, comment="#", skip_blank_lines=True - ).to_numpy() - # Save name and data - self.items[name] = (function_name, data) - values[name] = (function_name, data) - elif value == "[input]": - self.items[name] = pybamm.InputParameter(name) - # Anything else should be a converted to a float + filename = os.path.join(data_path, value[14:] + ".csv") + function_name = value[14:] else: - self.items[name] = float(value) - values[name] = float(value) + filename = os.path.join(path, value[6:] + ".csv") + function_name = value[6:] + data = pd.read_csv( + filename, comment="#", skip_blank_lines=True + ).to_numpy() + # Save name and data + self.items[name] = (function_name, data) + values[name] = (function_name, data) + elif value == "[input]": + self.items[name] = pybamm.InputParameter(name) + # Anything else should be a converted to a float else: - self.items[name] = value + self.items[name] = float(value) + values[name] = float(value) + else: + self.items[name] = value # check parameter values self.check_and_update_parameter_values(values) # reset processed symbols From 781dd8fc4617c296a662b2b743a31f9e55a0a625 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Thu, 23 Jan 2020 21:19:48 -0500 Subject: [PATCH 3/9] #790 speed up update --- pybamm/parameters/parameter_values.py | 7 ++---- .../test_asymptotics_convergence.py | 24 ++++++++++++------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py index ac8c3eb4d7..192f066e63 100644 --- a/pybamm/parameters/parameter_values.py +++ b/pybamm/parameters/parameter_values.py @@ -547,13 +547,10 @@ def update_scalars(self, symbol): for x in symbol.pre_order(): if isinstance(x, pybamm.Scalar): # update any Scalar nodes if their name is in the parameter dict - try: - x.value = self[x.name] + if x.name in self.items.keys(): + x.value = self.items[x.name] # update id x.set_id() - except KeyError: - # KeyError -> name not in parameter dict, don't update - continue return symbol diff --git a/tests/integration/test_models/test_full_battery_models/test_lead_acid/test_asymptotics_convergence.py b/tests/integration/test_models/test_full_battery_models/test_lead_acid/test_asymptotics_convergence.py index 6b3b7eef40..8b4e16f037 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lead_acid/test_asymptotics_convergence.py +++ b/tests/integration/test_models/test_full_battery_models/test_lead_acid/test_asymptotics_convergence.py @@ -17,8 +17,13 @@ def test_leading_order_convergence(self): leading_order_model = pybamm.lead_acid.LOQS() composite_model = pybamm.lead_acid.Composite() full_model = pybamm.lead_acid.Full() + + def current_function(t): + return pybamm.InputParameter("Current") + # Same parameters, same geometry parameter_values = full_model.default_parameter_values + parameter_values["Current function [A]"] = current_function parameter_values.process_model(leading_order_model) parameter_values.process_model(composite_model) parameter_values.process_model(full_model) @@ -44,26 +49,26 @@ def test_leading_order_convergence(self): def get_max_error(current): pybamm.logger.info("current = {}".format(current)) - # Update current (and hence C_e) in the parameters - param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Sulzer2019) - param.update({"Current function [A]": current}) - param.update_model(leading_order_model, loqs_disc) - param.update_model(composite_model, comp_disc) - param.update_model(full_model, full_disc) # Solve, make sure times are the same and use tight tolerances t_eval = np.linspace(0, 0.6) solver_loqs = leading_order_model.default_solver solver_loqs.rtol = 1e-8 solver_loqs.atol = 1e-8 - solution_loqs = solver_loqs.solve(leading_order_model, t_eval) + solution_loqs = solver_loqs.solve( + leading_order_model, t_eval, inputs={"Current": current} + ) solver_comp = composite_model.default_solver solver_comp.rtol = 1e-8 solver_comp.atol = 1e-8 - solution_comp = solver_comp.solve(composite_model, t_eval) + solution_comp = solver_comp.solve( + composite_model, t_eval, inputs={"Current": current} + ) solver_full = full_model.default_solver solver_full.rtol = 1e-8 solver_full.atol = 1e-8 - solution_full = solver_full.solve(full_model, t_eval) + solution_full = solver_full.solve( + full_model, t_eval, inputs={"Current": current} + ) # Post-process variables voltage_loqs = solution_loqs["Terminal voltage"] @@ -100,4 +105,5 @@ def get_max_error(current): if "-v" in sys.argv: debug = True + pybamm.set_logging_level("DEBUG") unittest.main() From bc5919c467b6d4555119fc4c275cbbf9481854b1 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Thu, 23 Jan 2020 23:15:10 -0500 Subject: [PATCH 4/9] #790 fix tests and examples --- examples/notebooks/change-settings.ipynb | 58 +++++++++++++------ examples/notebooks/parameter-values.ipynb | 16 ++--- examples/scripts/DFN.py | 2 +- examples/scripts/compare_lead_acid_3D.py | 1 - pybamm/models/base_model.py | 2 +- .../full_battery_models/base_battery_model.py | 2 +- pybamm/parameters/parameter_values.py | 51 +++++++++------- pybamm/parameters/print_parameters.py | 2 +- .../test_parameters/test_parameter_values.py | 6 +- 9 files changed, 87 insertions(+), 53 deletions(-) diff --git a/examples/notebooks/change-settings.ipynb b/examples/notebooks/change-settings.ipynb index ddbd895682..5ff3129cc7 100644 --- a/examples/notebooks/change-settings.ipynb +++ b/examples/notebooks/change-settings.ipynb @@ -123,6 +123,26 @@ "cell_type": "code", "execution_count": 3, "metadata": {}, + "outputs": [ + { + "data": { + "text/plain": [ + ">" + ] + }, + "execution_count": 3, + "metadata": {}, + "output_type": "execute_result" + } + ], + "source": [ + "model.default_parameter_values.items" + ] + }, + { + "cell_type": "code", + "execution_count": 4, + "metadata": {}, "outputs": [ { "name": "stdout", @@ -155,8 +175,8 @@ "Typical current [A] 0.680616\n", "Negative electrode conductivity [S.m-1] 100.0\n", "Maximum concentration in negative electrode [mol.m-3] 24983.2619938437\n", - "Negative electrode diffusivity [m2.s-1] \n", - "Negative electrode OCP [V] \n", + "Negative electrode diffusivity [m2.s-1] \n", + "Negative electrode OCP [V] \n", "Negative electrode porosity 0.3\n", "Negative electrode active material volume fraction 0.7\n", "Negative particle radius [m] 1e-05\n", @@ -173,15 +193,15 @@ "Negative electrode density [kg.m-3] 1657.0\n", "Negative electrode specific heat capacity [J.kg-1.K-1] 700.0\n", "Negative electrode thermal conductivity [W.m-1.K-1] 1.7\n", - "Negative electrode OCP entropic change [V.K-1] \n", + "Negative electrode OCP entropic change [V.K-1] \n", "Reference temperature [K] 298.15\n", - "Negative electrode reaction rate \n", + "Negative electrode reaction rate \n", "Negative reaction rate activation energy [J.mol-1] 37480.0\n", "Negative solid diffusion activation energy [J.mol-1] 42770.0\n", "Positive electrode conductivity [S.m-1] 10.0\n", "Maximum concentration in positive electrode [mol.m-3] 51217.9257309275\n", - "Positive electrode diffusivity [m2.s-1] \n", - "Positive electrode OCP [V] \n", + "Positive electrode diffusivity [m2.s-1] \n", + "Positive electrode OCP [V] \n", "Positive electrode porosity 0.3\n", "Positive electrode active material volume fraction 0.7\n", "Positive particle radius [m] 1e-05\n", @@ -198,8 +218,8 @@ "Positive electrode density [kg.m-3] 3262.0\n", "Positive electrode specific heat capacity [J.kg-1.K-1] 700.0\n", "Positive electrode thermal conductivity [W.m-1.K-1] 2.1\n", - "Positive electrode OCP entropic change [V.K-1] \n", - "Positive electrode reaction rate \n", + "Positive electrode OCP entropic change [V.K-1] \n", + "Positive electrode reaction rate \n", "Positive reaction rate activation energy [J.mol-1] 39570.0\n", "Positive solid diffusion activation energy [J.mol-1] 18550.0\n", "Separator porosity 1.0\n", @@ -210,8 +230,8 @@ "Separator thermal conductivity [W.m-1.K-1] 0.16\n", "Typical electrolyte concentration [mol.m-3] 1000.0\n", "Cation transference number 0.4\n", - "Electrolyte diffusivity [m2.s-1] \n", - "Electrolyte conductivity [S.m-1] \n", + "Electrolyte diffusivity [m2.s-1] \n", + "Electrolyte conductivity [S.m-1] \n", "Electrolyte diffusion activation energy [J.mol-1] 37040.0\n", "Electrolyte conductivity activation energy [J.mol-1] 34700.0\n", "Heat transfer coefficient [W.m-2.K-1] 10.0\n", @@ -250,7 +270,7 @@ }, { "cell_type": "code", - "execution_count": 4, + "execution_count": 5, "metadata": {}, "outputs": [ { @@ -282,7 +302,7 @@ }, { "cell_type": "code", - "execution_count": 5, + "execution_count": 6, "metadata": {}, "outputs": [], "source": [ @@ -298,7 +318,7 @@ }, { "cell_type": "code", - "execution_count": 6, + "execution_count": 7, "metadata": {}, "outputs": [], "source": [ @@ -314,7 +334,7 @@ }, { "cell_type": "code", - "execution_count": 7, + "execution_count": 8, "metadata": {}, "outputs": [ { @@ -352,7 +372,7 @@ }, { "cell_type": "code", - "execution_count": 8, + "execution_count": 9, "metadata": {}, "outputs": [ { @@ -384,7 +404,7 @@ }, { "cell_type": "code", - "execution_count": 9, + "execution_count": 10, "metadata": {}, "outputs": [], "source": [ @@ -408,7 +428,7 @@ }, { "cell_type": "code", - "execution_count": 10, + "execution_count": 11, "metadata": {}, "outputs": [ { @@ -453,7 +473,7 @@ }, { "cell_type": "code", - "execution_count": 11, + "execution_count": 12, "metadata": {}, "outputs": [ { @@ -477,7 +497,7 @@ }, { "cell_type": "code", - "execution_count": 12, + "execution_count": 13, "metadata": {}, "outputs": [ { diff --git a/examples/notebooks/parameter-values.ipynb b/examples/notebooks/parameter-values.ipynb index e6e5d8f35a..d4a12a04c0 100644 --- a/examples/notebooks/parameter-values.ipynb +++ b/examples/notebooks/parameter-values.ipynb @@ -47,7 +47,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "parameter values are {'a': 1, 'b': 2, 'c': 3}\n" + "parameter values are \n" ] } ], @@ -73,7 +73,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "parameter values are {'a': 4, 'b': 5, 'c': 6}\n" + "parameter values are \n" ] } ], @@ -126,7 +126,7 @@ "cell_type": "markdown", "metadata": {}, "source": [ - "We can input functions into the parameter values, either directly" + "We can input functions into the parameter values, either directly (note we bypass the check that the parameter already exists)" ] }, { @@ -138,14 +138,14 @@ "name": "stdout", "output_type": "stream", "text": [ - "parameter values are {'a': 4, 'b': 5, 'c': 6, 'cube function': }\n" + "parameter values are \n" ] } ], "source": [ "def cubed(x):\n", " return x ** 3\n", - "parameter_values.update({\"cube function\": cubed})\n", + "parameter_values.update({\"cube function\": cubed}, check_already_exists=False)\n", "print(\"parameter values are {}\".format(parameter_values))" ] }, @@ -165,7 +165,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "parameter values are {'a': 4, 'b': 5, 'c': 6, 'cube function': , 'square function': }\n" + "parameter values are \n" ] } ], @@ -178,7 +178,7 @@ "\"\"\"\n", ")\n", "f.close()\n", - "parameter_values.update({\"square function\": pybamm.load_function(\"squared.py\")})\n", + "parameter_values.update({\"square function\": pybamm.load_function(\"squared.py\")}, check_already_exists=False)\n", "print(\"parameter values are {}\".format(parameter_values))" ] }, @@ -405,7 +405,7 @@ { "data": { "text/plain": [ - "{Variable(-0x53753a8d5f6f95de, u, children=[], domain=[], auxiliary_domains={}): Multiplication(0x3e1bde14593f44cc, *, children=['-a', 'y[0:1]'], domain=[], auxiliary_domains={})}" + "{Variable(0x1b4aef6f668c631, u, children=[], domain=[], auxiliary_domains={}): Multiplication(-0x36caade7fdf02dac, *, children=['-a', 'y[0:1]'], domain=[], auxiliary_domains={})}" ] }, "execution_count": 12, diff --git a/examples/scripts/DFN.py b/examples/scripts/DFN.py index 12fc979513..4811df8a19 100644 --- a/examples/scripts/DFN.py +++ b/examples/scripts/DFN.py @@ -15,7 +15,7 @@ # load parameter values and process model and geometry param = model.default_parameter_values -param["Voltage function [V]"] = 4.1 +param.update({"Voltage function [V]": 4.1}, check_already_exists=False) param.process_model(model) param.process_geometry(geometry) diff --git a/examples/scripts/compare_lead_acid_3D.py b/examples/scripts/compare_lead_acid_3D.py index 964f0d1809..5b3f0e93bc 100644 --- a/examples/scripts/compare_lead_acid_3D.py +++ b/examples/scripts/compare_lead_acid_3D.py @@ -46,7 +46,6 @@ param.update( { "Current function [A]": 1, - "Bruggeman coefficient": 0.001, "Initial State of Charge": 1, "Typical electrolyte concentration [mol.m-3]": 5600, "Negative electrode reference exchange-current density [A.m-2]": 0.08, diff --git a/pybamm/models/base_model.py b/pybamm/models/base_model.py index 073b4e1887..0f5ff41aa5 100644 --- a/pybamm/models/base_model.py +++ b/pybamm/models/base_model.py @@ -208,7 +208,7 @@ def variables(self): @variables.setter def variables(self, variables): - self._variables.update(variables) + self._variables = pybamm.FuzzyDict(variables) def variable_names(self): return list(self._variables.keys()) diff --git a/pybamm/models/full_battery_models/base_battery_model.py b/pybamm/models/full_battery_models/base_battery_model.py index 08b764e946..d8c17e2416 100644 --- a/pybamm/models/full_battery_models/base_battery_model.py +++ b/pybamm/models/full_battery_models/base_battery_model.py @@ -341,7 +341,7 @@ def set_standard_output_variables(self): } ) - self._variables = pybamm.FuzzyDict() + self.variables = {} # Time time_scale = pybamm.electrical_parameters.timescale diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py index 192f066e63..b2a2e6ab2d 100644 --- a/pybamm/parameters/parameter_values.py +++ b/pybamm/parameters/parameter_values.py @@ -9,7 +9,7 @@ class ParameterValues: """ - The parameter values for a simulation. + The parameter values for a simulation. Note that this class does not inherit directly from the python dictionary class as this causes issues with saving and loading simulations. @@ -52,7 +52,7 @@ class ParameterValues: """ def __init__(self, values=None, chemistry=None): - self.items = pybamm.FuzzyDict() + self._dict_items = pybamm.FuzzyDict() # Must provide either values or chemistry, not both (nor neither) if values is not None and chemistry is not None: raise ValueError( @@ -80,18 +80,26 @@ def __init__(self, values=None, chemistry=None): self._processed_symbols = {} def __getitem__(self, key): - return self.items[key] + return self._dict_items[key] def __setitem__(self, key, value): "Call the update functionality when doing a setitem" self.update({key: value}) def __delitem__(self, key): - del self.items[key] + del self._dict_items[key] def keys(self): "Get the keys of the dictionary" - return self.items.keys() + return self._dict_items.keys() + + def values(self): + "Get the values of the dictionary" + return self._dict_items.values() + + def items(self): + "Get the items of the dictionary" + return self._dict_items.items() def update_from_chemistry(self, chemistry): """ @@ -167,12 +175,15 @@ def update(self, values, check_conflict=False, check_already_exists=True, path=" # check parameter already exists (for updating parameters) if check_already_exists is True: try: - self.items[name] + self._dict_items[name] except KeyError as err: raise KeyError( - "cannot update parameter '{}' ".format(name) - + "as it does not have a default value. ({})".format( - err.args[0] + """ + Cannot update parameter '{}' as it does not have a default + value. ({}). If you are sure you want to update this parameter, + use param.update({{name: value}}, check_already_exists=False) + """.format( + name, err.args[0] ) ) # if no conflicts, update, loading functions and data if they are specified @@ -182,7 +193,7 @@ def update(self, values, check_conflict=False, check_already_exists=True, path=" loaded_value = pybamm.load_function( os.path.join(path, value[10:] + ".py") ) - self.items[name] = loaded_value + self._dict_items[name] = loaded_value values[name] = loaded_value # Data is flagged with the string "[data]" or "[current data]" elif value.startswith("[current data]") or value.startswith("[data]"): @@ -199,16 +210,16 @@ def update(self, values, check_conflict=False, check_already_exists=True, path=" filename, comment="#", skip_blank_lines=True ).to_numpy() # Save name and data - self.items[name] = (function_name, data) + self._dict_items[name] = (function_name, data) values[name] = (function_name, data) elif value == "[input]": - self.items[name] = pybamm.InputParameter(name) + self._dict_items[name] = pybamm.InputParameter(name) # Anything else should be a converted to a float else: - self.items[name] = float(value) + self._dict_items[name] = float(value) values[name] = float(value) else: - self.items[name] = value + self._dict_items[name] = value # check parameter values self.check_and_update_parameter_values(values) # reset processed symbols @@ -231,12 +242,12 @@ def check_and_update_parameter_values(self, values): ) # If the capacity of the cell has been provided, make sure "C-rate" and current # match with the stated capacity - if "Cell capacity [A.h]" in values or "Cell capacity [A.h]" in self.items: + if "Cell capacity [A.h]" in values or "Cell capacity [A.h]" in self._dict_items: # Capacity from values takes precedence if "Cell capacity [A.h]" in values: capacity = values["Cell capacity [A.h]"] else: - capacity = self.items["Cell capacity [A.h]"] + capacity = self._dict_items["Cell capacity [A.h]"] # Make sure they match if both provided # Update the other if only one provided if "C-rate" in values: @@ -249,7 +260,7 @@ def check_and_update_parameter_values(self, values): value = (values["C-rate"][0] + "_to_Crate", data) else: value = values["C-rate"] * capacity - self.items["Current function [A]"] = value + self._dict_items["Current function [A]"] = value elif "Current function [A]" in values: if callable(values["Current function [A]"]): value = CurrentToCrate(values["Current function [A]"], capacity) @@ -259,7 +270,7 @@ def check_and_update_parameter_values(self, values): value = (values["Current function [A]"][0] + "_to_current", data) else: value = values["Current function [A]"] / capacity - self.items["C-rate"] = value + self._dict_items["C-rate"] = value return values @@ -547,8 +558,8 @@ def update_scalars(self, symbol): for x in symbol.pre_order(): if isinstance(x, pybamm.Scalar): # update any Scalar nodes if their name is in the parameter dict - if x.name in self.items.keys(): - x.value = self.items[x.name] + if x.name in self._dict_items.keys(): + x.value = self._dict_items[x.name] # update id x.set_id() diff --git a/pybamm/parameters/print_parameters.py b/pybamm/parameters/print_parameters.py index 775df80043..381c3f369d 100644 --- a/pybamm/parameters/print_parameters.py +++ b/pybamm/parameters/print_parameters.py @@ -61,7 +61,7 @@ def print_parameters(parameters, parameter_values, output_file=None): # Calculate parameters for each C-rate for Crate in [1, 10]: # Update Crate - parameter_values.update({"C-rate": Crate}) + parameter_values.update({"C-rate": Crate}, check_already_exists=False) for name, symbol in parameters.items(): if not callable(symbol): proc_symbol = parameter_values.process_symbol(symbol) diff --git a/tests/unit/test_parameters/test_parameter_values.py b/tests/unit/test_parameters/test_parameter_values.py index c24a1f6a41..91e032984d 100644 --- a/tests/unit/test_parameters/test_parameter_values.py +++ b/tests/unit/test_parameters/test_parameter_values.py @@ -20,6 +20,10 @@ def test_init(self): # from dict param = pybamm.ParameterValues({"a": 1}) self.assertEqual(param["a"], 1) + self.assertEqual(list(param.keys())[0], "a") + self.assertEqual(list(param.values())[0], 1) + self.assertEqual(list(param.items())[0], ("a", 1)) + # from file param = pybamm.ParameterValues( values="input/parameters/lithium-ion/cathodes/lico2_Marquis2019/" @@ -58,7 +62,7 @@ def test_update(self): ): param.update({"a": 4}, check_conflict=True) # with parameter not existing yet - with self.assertRaisesRegex(KeyError, "cannot update parameter"): + with self.assertRaisesRegex(KeyError, "Cannot update parameter"): param.update({"b": 1}) def test_check_and_update_parameter_values(self): From 3df07701ffa3295a29c86e34e23717511fb5f789 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Thu, 23 Jan 2020 23:52:47 -0500 Subject: [PATCH 5/9] #790 another test and flake8 --- tests/unit/test_parameters/test_parameter_values.py | 2 +- tests/unit/test_simulation.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_parameters/test_parameter_values.py b/tests/unit/test_parameters/test_parameter_values.py index 91e032984d..976b595aec 100644 --- a/tests/unit/test_parameters/test_parameter_values.py +++ b/tests/unit/test_parameters/test_parameter_values.py @@ -23,7 +23,7 @@ def test_init(self): self.assertEqual(list(param.keys())[0], "a") self.assertEqual(list(param.values())[0], 1) self.assertEqual(list(param.items())[0], ("a", 1)) - + # from file param = pybamm.ParameterValues( values="input/parameters/lithium-ion/cathodes/lico2_Marquis2019/" diff --git a/tests/unit/test_simulation.py b/tests/unit/test_simulation.py index 4dc0a8f103..a65177d6bc 100644 --- a/tests/unit/test_simulation.py +++ b/tests/unit/test_simulation.py @@ -295,7 +295,8 @@ def test_set_defaults2(self): # Not sure of best way to test nested dicts? # self.geometry = model.default_geometry self.assertEqual( - sim._parameter_values.items, model.default_parameter_values.items + sim._parameter_values._dict_items, + model.default_parameter_values._dict_items, ) for domain, submesh in model.default_submesh_types.items(): self.assertEqual( From b63f8ca6a596ba3879ab357c79cf1a6194fefccf Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Thu, 23 Jan 2020 23:55:56 -0500 Subject: [PATCH 6/9] #790 changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 834c1502fd..b6599563de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Features +- Added fuzzy string matching for parameters and variables ([#796](https://github.com/pybamm-team/PyBaMM/pull/796)) +- Changed ParameterValues to raise an error when a parameter that wasn't previously defined is updated ([#796](https://github.com/pybamm-team/PyBaMM/pull/796)) - Added the harmonic mean to the Finite Volume method, which is now used when computing fluxes ([#783](https://github.com/pybamm-team/PyBaMM/pull/783)) - Refactored `Solution` to make it a dictionary that contains all of the solution variables. This automatically creates `ProcessedVariable` objects when required, so that the solution can be obtained much more easily. ([#781](https://github.com/pybamm-team/PyBaMM/pull/781)) - Added notebook to explain broadcasts ([#776](https://github.com/pybamm-team/PyBaMM/pull/776)) From 0ee0e8f0e4237da4ee891c1254eb60026737e4a9 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Fri, 24 Jan 2020 08:30:37 -0500 Subject: [PATCH 7/9] #790 coverage --- tests/unit/test_parameters/test_parameter_values.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_parameters/test_parameter_values.py b/tests/unit/test_parameters/test_parameter_values.py index 976b595aec..893e5a6d90 100644 --- a/tests/unit/test_parameters/test_parameter_values.py +++ b/tests/unit/test_parameters/test_parameter_values.py @@ -56,9 +56,11 @@ def test_update(self): self.assertEqual(param["a"], 2) # with conflict param.update({"a": 3}) - self.assertEqual(param["a"], 3) + # via __setitem__ + param["a"] = 2 + self.assertEqual(param["a"], 2) with self.assertRaisesRegex( - ValueError, "parameter 'a' already defined with value '3'" + ValueError, "parameter 'a' already defined with value '2'" ): param.update({"a": 4}, check_conflict=True) # with parameter not existing yet From 02ef8f7ca13ffffdb0e9203a182e654d8f551108 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Mon, 27 Jan 2020 23:39:11 -0500 Subject: [PATCH 8/9] #790 remove fuzzy module --- .requirements-docs.txt | 1 - pybamm/util.py | 31 ++++++++++++++++++++++++++++--- setup.py | 1 - 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/.requirements-docs.txt b/.requirements-docs.txt index 1f9addea77..6dbc42b983 100644 --- a/.requirements-docs.txt +++ b/.requirements-docs.txt @@ -8,5 +8,4 @@ scikit-fem>=0.2.0 casadi>=3.5.0 guzzle-sphinx-theme sphinx>=1.5 -fuzzywuzzy>=0.1.0 python-Levenshtein>=0.12.0 diff --git a/pybamm/util.py b/pybamm/util.py index 16042bd1f8..045b85f8ce 100644 --- a/pybamm/util.py +++ b/pybamm/util.py @@ -12,8 +12,8 @@ import pathlib import pickle import pybamm +import Levenshtein from collections import defaultdict -from fuzzywuzzy import process def root_dir(): @@ -22,12 +22,36 @@ def root_dir(): class FuzzyDict(dict): + def get_best_matches(self, key): + "Get best matches from keys" + key = key.lower() + best_three = [] + lowest_score = 0 + for k in self.keys(): + score = Levenshtein.ratio(k.lower(), key) + # Start filling out the list + if len(best_three) < 3: + best_three.append((k, score)) + # Sort once the list has three elements, using scores + if len(best_three) == 3: + best_three.sort(key=lambda x: x[1], reverse=True) + lowest_score = best_three[-1][1] + # Once list is full, start checking new entries + else: + if score > lowest_score: + # Replace last element with new entry + best_three[-1] = (k, score) + # Sort and update lowest score + best_three.sort(key=lambda x: x[1], reverse=True) + lowest_score = best_three[-1][1] + + return [x[0] for x in best_three] + def __getitem__(self, key): try: return super().__getitem__(key) except KeyError: - best_matches = process.extract(key, self.keys(), limit=3) - best_matches = [match[0] for match in best_matches] + best_matches = self.get_best_matches(key) raise KeyError(f"'{key}' not found. Best matches are {best_matches}") @@ -205,3 +229,4 @@ def load(filename): with open(filename, "rb") as f: obj = pickle.load(f) return obj + diff --git a/setup.py b/setup.py index 29a517b247..b7672fcacc 100644 --- a/setup.py +++ b/setup.py @@ -48,7 +48,6 @@ def load_version(): "scikit-fem>=0.2.0", "casadi>=3.5.0", "jupyter", # For example notebooks - "fuzzywuzzy>=0.17.0", "python-Levenshtein>=0.12.0", # Note: Matplotlib is loaded for debug plots, but to ensure pybamm runs # on systems without an attached display, it should never be imported From 1d10e77eea33ef778fc6596e44b3aed53b3d0ce1 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Tue, 28 Jan 2020 08:45:09 -0500 Subject: [PATCH 9/9] #790 doc new parameter values --- pybamm/parameters/parameter_values.py | 20 ++++++++++++++++++++ tests/unit/test_simulation.py | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py index b2a2e6ab2d..57a7f4fb35 100644 --- a/pybamm/parameters/parameter_values.py +++ b/pybamm/parameters/parameter_values.py @@ -159,6 +159,26 @@ def read_parameters_csv(self, filename): return {k: v for (k, v) in zip(df["Name [units]"], df["Value"])} def update(self, values, check_conflict=False, check_already_exists=True, path=""): + """ + Update parameter dictionary, while also performing some basic checks. + + Parameters + ---------- + values : dict + Dictionary of parameter values to update parameter dictionary with + check_conflict : bool, optional + Whether to check that a parameter in `values` has not already been defined + in the parameter class when updating it, and if so that its value does not + change. This is set to True during initialisation, when parameters are + combined from different sources, and is False by default otherwise + check_already_exists : bool, optional + Whether to check that a parameter in `values` already exists when trying to + update it. This is to avoid cases where an intended change in the parameters + is ignored due a typo in the parameter name, and is True by default but can + be manually overridden. + path : string, optional + Path from which to load functions + """ # update for name, value in values.items(): # check for conflicts diff --git a/tests/unit/test_simulation.py b/tests/unit/test_simulation.py index 629058bd96..0b1a8b4e2c 100644 --- a/tests/unit/test_simulation.py +++ b/tests/unit/test_simulation.py @@ -230,7 +230,7 @@ def current_function(t): dt = 0.001 model = pybamm.lithium_ion.SPM() param = model.default_parameter_values - param.update({"Current function [A]": current_function, "Current": "[input]"}) + param.update({"Current function [A]": current_function}) sim = pybamm.Simulation(model, parameter_values=param) sim.step(dt, inputs={"Current": 1}) # 1 step stores first two points self.assertEqual(sim.solution.t.size, 2)