diff --git a/examples/scripts/compare_comsol/compare_comsol_DFN.py b/examples/scripts/compare_comsol/compare_comsol_DFN.py index da6121c4be..f11d26245f 100644 --- a/examples/scripts/compare_comsol/compare_comsol_DFN.py +++ b/examples/scripts/compare_comsol/compare_comsol_DFN.py @@ -128,7 +128,8 @@ def myinterp(t): "Positive electrode potential [V]": comsol_phi_p, "Terminal voltage [V]": comsol_voltage, } -comsol_solution = pybamm.CasadiSolver(mode="fast").solve(pybamm_model, time) +# Make new solution with same t and y +comsol_solution = pybamm.Solution(pybamm_solution.t, pybamm_solution.y) comsol_solution.model = comsol_model # plot plot = pybamm.QuickPlot( diff --git a/examples/scripts/compare_comsol/discharge_curve.py b/examples/scripts/compare_comsol/discharge_curve.py index 33e3b14c56..110cfc5dbd 100644 --- a/examples/scripts/compare_comsol/discharge_curve.py +++ b/examples/scripts/compare_comsol/discharge_curve.py @@ -14,10 +14,16 @@ model = pybamm.lithium_ion.DFN() geometry = model.default_geometry + # load parameters and process model and geometry +def current_function(t): + return pybamm.InputParameter("current") + + param = model.default_parameter_values param["Electrode width [m]"] = 1 param["Electrode height [m]"] = 1 +param["Current function [A]"] = current_function param.process_model(model) param.process_geometry(geometry) @@ -48,7 +54,7 @@ plt.ylabel(r"$\vert V - V_{comsol} \vert$", fontsize=20) for key, C_rate in C_rates.items(): - + current = 24 * C_rate # load the comsol results comsol_variables = pickle.load( open("input/comsol_results/comsol_{}C.pickle".format(key), "rb") @@ -57,8 +63,6 @@ comsol_voltage = comsol_variables["voltage"] # update current density - param["Current function [A]"] = 24 * C_rate - param.update_model(model, disc) # discharge timescale tau = param.process_symbol( @@ -67,12 +71,14 @@ # solve model at comsol times t = comsol_time / tau - solution = pybamm.CasadiSolver(mode="fast").solve(model, t) + solution = pybamm.CasadiSolver(mode="fast").solve( + model, t, inputs={"current": current} + ) # discharge capacity discharge_capacity = solution["Discharge capacity [A.h]"] discharge_capacity_sol = discharge_capacity(solution.t) - comsol_discharge_capacity = comsol_time * param["Current function [A]"] / 3600 + comsol_discharge_capacity = comsol_time * current / 3600 # extract the voltage voltage = solution["Terminal voltage [V]"] diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py index 57a7f4fb35..ace705420e 100644 --- a/pybamm/parameters/parameter_values.py +++ b/pybamm/parameters/parameter_values.py @@ -294,7 +294,7 @@ def check_and_update_parameter_values(self, values): return values - def process_model(self, unprocessed_model, processing="process", inplace=True): + def process_model(self, unprocessed_model, inplace=True): """Assign parameter values to a model. Currently inplace, could be changed to return a new model. @@ -302,13 +302,6 @@ def process_model(self, unprocessed_model, processing="process", inplace=True): ---------- unprocessed_model : :class:`pybamm.BaseModel` Model to assign parameter values for - processing : str, optional - Flag to indicate how to process model (default 'process') - - * 'process': Calls :meth:`process_symbol()` (walk through the symbol \ - and replace any Parameter with a Value) - * 'update': Calls :meth:`update_scalars()` for use on already-processed \ - model (update the value of any Scalars in the expression tree.) inplace: bool, optional If True, replace the parameters in the model in place. Otherwise, return a new model with parameter values set. Default is True. @@ -335,32 +328,21 @@ def process_model(self, unprocessed_model, processing="process", inplace=True): if len(unprocessed_model.rhs) == 0 and len(unprocessed_model.algebraic) == 0: raise pybamm.ModelError("Cannot process parameters for empty model") - if processing == "process": - processing_function = self.process_symbol - elif processing == "update": - processing_function = self.update_scalars - for variable, equation in model.rhs.items(): - pybamm.logger.debug( - "{} parameters for {!r} (rhs)".format(processing.capitalize(), variable) - ) - model.rhs[variable] = processing_function(equation) + pybamm.logger.debug("Processing parameters for {!r} (rhs)".format(variable)) + model.rhs[variable] = self.process_symbol(equation) for variable, equation in model.algebraic.items(): pybamm.logger.debug( - "{} parameters for {!r} (algebraic)".format( - processing.capitalize(), variable - ) + "Processing parameters for {!r} (algebraic)".format(variable) ) - model.algebraic[variable] = processing_function(equation) + model.algebraic[variable] = self.process_symbol(equation) for variable, equation in model.initial_conditions.items(): pybamm.logger.debug( - "{} parameters for {!r} (initial conditions)".format( - processing.capitalize(), variable - ) + "Processing parameters for {!r} (initial conditions)".format(variable) ) - model.initial_conditions[variable] = processing_function(equation) + model.initial_conditions[variable] = self.process_symbol(equation) # Boundary conditions are dictionaries {"left": left bc, "right": right bc} # in general, but may be imposed on the tabs (or *not* on the tab) for a @@ -369,17 +351,15 @@ def process_model(self, unprocessed_model, processing="process", inplace=True): new_boundary_conditions = {} sides = ["left", "right", "negative tab", "positive tab", "no tab"] for variable, bcs in model.boundary_conditions.items(): - processed_variable = processing_function(variable) + processed_variable = self.process_symbol(variable) new_boundary_conditions[processed_variable] = {} for side in sides: try: bc, typ = bcs[side] pybamm.logger.debug( - "{} parameters for {!r} ({} bc)".format( - processing.capitalize(), variable, side - ) + "Processing parameters for {!r} ({} bc)".format(variable, side) ) - processed_bc = (processing_function(bc), typ) + processed_bc = (self.process_symbol(bc), typ) new_boundary_conditions[processed_variable][side] = processed_bc except KeyError as err: # don't raise error if the key error comes from the side not being @@ -394,42 +374,24 @@ def process_model(self, unprocessed_model, processing="process", inplace=True): for variable, equation in model.variables.items(): pybamm.logger.debug( - "{} parameters for {!r} (variables)".format( - processing.capitalize(), variable - ) + "Processing parameters for {!r} (variables)".format(variable) ) - model.variables[variable] = processing_function(equation) + model.variables[variable] = self.process_symbol(equation) for event, equation in model.events.items(): - pybamm.logger.debug( - "{} parameters for event '{}''".format(processing.capitalize(), event) - ) - model.events[event] = processing_function(equation) + pybamm.logger.debug("Processing parameters for event '{}''".format(event)) + model.events[event] = self.process_symbol(equation) pybamm.logger.info("Finish setting parameters for {}".format(model.name)) return model def update_model(self, model, disc): - """Process a discretised model. - Currently inplace, could be changed to return a new model. - - Parameters - ---------- - model : :class:`pybamm.BaseModel` - Model to assign parameter values for - disc : :class:`pybamm.Discretisation` - The class that was used to discretise - - """ - # process parameter values for the model - self.process_model(model, processing="update") - - # update discretised quantities using disc - model.concatenated_rhs = disc._concatenate_in_order(model.rhs) - model.concatenated_algebraic = disc._concatenate_in_order(model.algebraic) - model.concatenated_initial_conditions = disc._concatenate_in_order( - model.initial_conditions - ).evaluate(0, None) + raise NotImplementedError( + """ + update_model functionality has been deprecated. + Use pybamm.InputParameter to quickly change a parameter value instead + """ + ) def process_geometry(self, geometry): """ @@ -561,30 +523,6 @@ def _process_symbol(self, symbol): ) ) - def update_scalars(self, symbol): - """Update the value of any Scalars in the expression tree. - - Parameters - ---------- - symbol : :class:`pybamm.Symbol` - Symbol or Expression tree to update - - Returns - ------- - symbol : :class:`pybamm.Symbol` - Symbol with Scalars updated - - """ - 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._dict_items.keys(): - x.value = self._dict_items[x.name] - # update id - x.set_id() - - return symbol - def evaluate(self, symbol): """ Process and evaluate a symbol. diff --git a/tests/integration/test_models/standard_model_tests.py b/tests/integration/test_models/standard_model_tests.py index ba827e9cd3..1f1340730d 100644 --- a/tests/integration/test_models/standard_model_tests.py +++ b/tests/integration/test_models/standard_model_tests.py @@ -103,28 +103,6 @@ def test_all( ): self.test_outputs() - def test_update_parameters(self, param): - # check if geometry has changed, throw error if so (need to re-discretise) - if any( - [ - length in param.keys() - and param[length] != self.parameter_values[length] - for length in [ - "Negative electrode thickness [m]", - "Separator thickness [m]", - "Positive electrode thickness [m]", - ] - ] - ): - raise ValueError( - "geometry has changed, the orginal model needs to be re-discretised" - ) - # otherwise update self.param and change the parameters in the discretised model - self.param = param - param.update_model(self.model, self.disc) - # Model should still be well-posed after processing - self.model.check_well_posedness(post_discretisation=True) - class OptimisationsTest(object): """ Test that the optimised models give the same result as the original model. """ diff --git a/tests/integration/test_quick_plot.py b/tests/integration/test_quick_plot.py index 582503c334..c18e0b3d67 100644 --- a/tests/integration/test_quick_plot.py +++ b/tests/integration/test_quick_plot.py @@ -41,13 +41,6 @@ def test_plot_lithium_ion(self): quick_plot.update(0.01) - # Update parameters, solve, plot again - param.update({"Current function [A]": 0}) - param.update_model(spm, disc_spm) - solution_spm = spm.default_solver.solve(spm, t_eval) - quick_plot = pybamm.QuickPlot(solution_spm) - quick_plot.plot(0) - # Test with different output variables output_vars = [ "Negative particle surface concentration", diff --git a/tests/unit/test_parameters/test_parameter_values.py b/tests/unit/test_parameters/test_parameter_values.py index 893e5a6d90..5d6d4e9115 100644 --- a/tests/unit/test_parameters/test_parameter_values.py +++ b/tests/unit/test_parameters/test_parameter_values.py @@ -303,18 +303,18 @@ def test_process_inline_function_parameters(self): def D(c): return c ** 2 - parameter_values = pybamm.ParameterValues({"a": 3, "Diffusivity": D}) + parameter_values = pybamm.ParameterValues({"Diffusivity": D}) - a = pybamm.Parameter("a") + a = pybamm.InputParameter("a") func = pybamm.FunctionParameter("Diffusivity", a) processed_func = parameter_values.process_symbol(func) - self.assertEqual(processed_func.evaluate(), 9) + self.assertEqual(processed_func.evaluate(u={"a": 3}), 9) # process differentiated function parameter diff_func = func.diff(a) processed_diff_func = parameter_values.process_symbol(diff_func) - self.assertEqual(processed_diff_func.evaluate(), 6) + self.assertEqual(processed_diff_func.evaluate(u={"a": 3}), 6) def test_multi_var_function_with_parameters(self): def D(a, b): @@ -362,7 +362,7 @@ def test_process_interpolant(self): self.assertEqual(processed_diff_func.evaluate(), 2) def test_interpolant_against_function(self): - parameter_values = pybamm.ParameterValues({"a": 0.6}) + parameter_values = pybamm.ParameterValues({}) parameter_values.update( { "function": "[function]lico2_ocp_Dualfoil1998", @@ -379,14 +379,16 @@ def test_interpolant_against_function(self): check_already_exists=False, ) - a = pybamm.Parameter("a") + a = pybamm.InputParameter("a") func = pybamm.FunctionParameter("function", a) interp = pybamm.FunctionParameter("interpolation", a) processed_func = parameter_values.process_symbol(func) processed_interp = parameter_values.process_symbol(interp) np.testing.assert_array_almost_equal( - processed_func.evaluate(), processed_interp.evaluate(), decimal=4 + processed_func.evaluate(u={"a": 0.6}), + processed_interp.evaluate(u={"a": 0.6}), + decimal=4, ) # process differentiated function parameter @@ -395,7 +397,9 @@ def test_interpolant_against_function(self): processed_diff_func = parameter_values.process_symbol(diff_func) processed_diff_interp = parameter_values.process_symbol(diff_interp) np.testing.assert_array_almost_equal( - processed_diff_func.evaluate(), processed_diff_interp.evaluate(), decimal=2 + processed_diff_func.evaluate(u={"a": 0.6}), + processed_diff_interp.evaluate(u={"a": 0.6}), + decimal=2, ) def test_process_complex_expression(self): @@ -500,6 +504,17 @@ def test_process_model(self): with self.assertRaises(KeyError): parameter_values.process_model(model) + def test_inplace(self): + model = pybamm.lithium_ion.SPM() + param = model.default_parameter_values + new_model = param.process_model(model, inplace=False) + + for val in list(model.rhs.values()): + self.assertTrue(val.has_symbol_of_classes(pybamm.Parameter)) + + for val in list(new_model.rhs.values()): + self.assertFalse(val.has_symbol_of_classes(pybamm.Parameter)) + def test_process_empty_model(self): model = pybamm.BaseModel() parameter_values = pybamm.ParameterValues({"a": 1, "b": 2, "c": 3, "d": 42}) diff --git a/tests/unit/test_parameters/test_update_parameters.py b/tests/unit/test_parameters/test_update_parameters.py deleted file mode 100644 index 5193e44306..0000000000 --- a/tests/unit/test_parameters/test_update_parameters.py +++ /dev/null @@ -1,147 +0,0 @@ -# -# Tests for updating parameter values -# -import pybamm - -import unittest -import numpy as np -import tests - - -class TestUpdateParameters(unittest.TestCase): - def test_update_parameters_eqn(self): - a = pybamm.Scalar(1) - b = pybamm.Scalar(2, name="test parameter") - c = pybamm.Scalar(3) - eqn = a + b * c - self.assertEqual(eqn.evaluate(), 7) - - parameter_values = pybamm.ParameterValues({"test parameter": 3}) - eqn_changed = parameter_values.update_scalars(eqn) - self.assertEqual(eqn_changed.evaluate(), 10) - - def test_set_and_update_parameters(self): - a = pybamm.Scalar(1) - b = pybamm.Parameter(name="test parameter") - c = pybamm.Scalar(3) - eqn = a + b * c - - parameter_values = pybamm.ParameterValues({"test parameter": 2}) - eqn_processed = parameter_values.process_symbol(eqn) - self.assertEqual(eqn_processed.evaluate(), 7) - - parameter_values = pybamm.ParameterValues({"test parameter": 3}) - eqn_updated = parameter_values.update_scalars(eqn_processed) - self.assertEqual(eqn_updated.evaluate(), 10) - - def test_update_model(self): - # test on simple lithium-ion model - model1 = pybamm.lithium_ion.SPM() - modeltest1 = tests.StandardModelTest(model1) - t_eval = np.linspace(0, 0.1) - - modeltest1.test_all(t_eval=t_eval, skip_output_tests=True) - Y1 = modeltest1.solution.y - - # process and solve the model a first time - model2 = pybamm.lithium_ion.SPM() - modeltest2 = tests.StandardModelTest(model2) - modeltest2.test_all(skip_output_tests=True) - self.assertEqual(model2.variables["Current [A]"].evaluate(), 0.680616) - # process and solve with updated parameter values - parameter_values_update = pybamm.ParameterValues( - chemistry=pybamm.parameter_sets.Marquis2019 - ) - parameter_values_update.update({"Current function [A]": 1}) - modeltest2.test_update_parameters(parameter_values_update) - self.assertEqual(model2.variables["Current [A]"].evaluate(), 1) - modeltest2.test_solving(t_eval=t_eval) - Y2 = modeltest2.solution.y - - # results should be different - self.assertNotEqual(np.linalg.norm(Y1 - Y2), 0) - - # test with new current function - model3 = pybamm.lithium_ion.SPM() - modeltest3 = tests.StandardModelTest(model3) - modeltest3.test_all(skip_output_tests=True) - parameter_values_update = pybamm.ParameterValues( - chemistry=pybamm.parameter_sets.Marquis2019 - ) - parameter_values_update.update({"Current function [A]": 0}) - modeltest3.test_update_parameters(parameter_values_update) - modeltest3.test_solving(t_eval=t_eval) - Y3 = modeltest3.solution.y - - self.assertIsInstance(model3.variables["Current [A]"], pybamm.Multiplication) - self.assertIsInstance(model3.variables["Current [A]"].left, pybamm.Scalar) - self.assertIsInstance(model3.variables["Current [A]"].right, pybamm.Scalar) - self.assertEqual(model3.variables["Current [A]"].evaluate(), 0.0) - - # results should be different - self.assertNotEqual(np.linalg.norm(Y1 - Y3), 0) - - def test_inplace(self): - model = pybamm.lithium_ion.SPM() - param = model.default_parameter_values - new_model = param.process_model(model, inplace=False) - - for val in list(model.rhs.values()): - self.assertTrue(val.has_symbol_of_classes(pybamm.Parameter)) - - for val in list(new_model.rhs.values()): - self.assertFalse(val.has_symbol_of_classes(pybamm.Parameter)) - - def test_update_geometry(self): - # test on simple lead-acid model - model1 = pybamm.lead_acid.LOQS() - modeltest1 = tests.StandardModelTest(model1) - parameter_values = pybamm.ParameterValues( - chemistry=pybamm.parameter_sets.Sulzer2019 - ) - parameter_values.update({"C-rate": 0.05}) - t_eval = np.linspace(0, 0.5) - modeltest1.test_all( - param=parameter_values, t_eval=t_eval, skip_output_tests=True - ) - - # trying to update the geometry fails - parameter_values_update = pybamm.ParameterValues( - chemistry=pybamm.parameter_sets.Sulzer2019 - ) - parameter_values_update.update( - { - "C-rate": 0.05, - "Negative electrode thickness [m]": 0.0002, - "Separator thickness [m]": 0.0003, - "Positive electrode thickness [m]": 0.0004, - } - ) - with self.assertRaisesRegex(ValueError, "geometry has changed"): - modeltest1.test_update_parameters(parameter_values_update) - - # instead we need to make a new model and re-discretise - model2 = pybamm.lead_acid.LOQS() - # nb: need to be careful make parameters a reasonable size - modeltest2 = tests.StandardModelTest(model2) - modeltest2.test_all( - param=parameter_values_update, t_eval=t_eval, skip_output_tests=True - ) - # results should be different - c1 = modeltest1.solution["Electrolyte concentration"].entries - c2 = modeltest2.solution["Electrolyte concentration"].entries - self.assertNotEqual(np.linalg.norm(c1 - c2), 0) - self.assertNotEqual( - np.linalg.norm(modeltest1.solution.y - modeltest2.solution.y), 0 - ) - - -if __name__ == "__main__": - import sys - - print("Add -v for more debug output") - - if "-v" in sys.argv: - debug = True - pybamm.settings.debug_mode = True - unittest.main()