diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bfc4528..e1d01785 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ ## Bug Fixes +- [#233](https://github.com/pybop-team/PyBOP/pull/233) - Enforces model rebuild on initialisation of a Problem to allow a change of experiment, fixes if statement triggering current function update, updates `predictions` to `simulation` to keep distinction between `predict` and `simulate` and adds `test_changes`. - [#123](https://github.com/pybop-team/PyBOP/issues/123) - Reinstates check for availability of parameter sets via PyBaMM upon retrieval by `pybop.ParameterSet.pybamm()`. - [#196](https://github.com/pybop-team/PyBOP/issues/196) - Fixes failing observer cost tests. - [#63](https://github.com/pybop-team/PyBOP/issues/63) - Removes NLOpt Optimiser from future releases. This is to support deployment to the Apple M-Series platform. diff --git a/pybop/_experiment.py b/pybop/_experiment.py index e35a6acc..1c495384 100644 --- a/pybop/_experiment.py +++ b/pybop/_experiment.py @@ -1,7 +1,7 @@ -import pybamm +from pybamm import Experiment -class Experiment(pybamm.Experiment): +class Experiment(Experiment): """ Wraps the Experiment class for generating experiment conditions for PyBaMM models. Credit: PyBaMM diff --git a/pybop/_problem.py b/pybop/_problem.py index 9cae745a..5665d236 100644 --- a/pybop/_problem.py +++ b/pybop/_problem.py @@ -200,8 +200,13 @@ def __init__( self._model.n_outputs = self.n_outputs self._model.n_time_data = self.n_time_data - # Build the model - if self._model._built_model is None: + # Build the model from scratch + if self._model._built_model is not None: + self._model._model_with_set_params = None + self._model._built_model = None + self._model._built_initial_soc = None + self._model._mesh = None + self._model._disc = None self._model.build( dataset=self._dataset, parameters=self.parameters, diff --git a/pybop/models/base_model.py b/pybop/models/base_model.py index 1f56f9b4..5dde68c4 100644 --- a/pybop/models/base_model.py +++ b/pybop/models/base_model.py @@ -75,7 +75,7 @@ def build( Construct the PyBaMM model if not already built, and set parameters. This method initializes the model components, applies the given parameters, - sets up the mesh and discretization if needed, and prepares the model + sets up the mesh and discretisation if needed, and prepares the model for simulations. Parameters @@ -92,7 +92,7 @@ def build( self.dataset = dataset self.parameters = parameters if self.parameters is not None: - self.set_parameter_classification(self.parameters) + self.classify_and_update_parameters(self.parameters) self.fit_keys = [param.name for param in self.parameters] else: self.fit_keys = [] @@ -106,6 +106,7 @@ def build( elif self.pybamm_model.is_discretised: self._model_with_set_params = self.pybamm_model self._built_model = self.pybamm_model + else: self.set_params() @@ -156,11 +157,10 @@ def set_params(self, rebuild=False): return # Mark any simulation inputs in the parameter set - if self.non_matched_parameters: - for i in self.fit_keys: - self._parameter_set[i] = "[input]" + for key in self.non_matched_parameters.keys(): + self._parameter_set[key] = "[input]" - if self.dataset is not None and self.non_matched_parameters: + if self.dataset is not None and (not self.matched_parameters or not rebuild): if "Current function [A]" not in self.fit_keys: self._parameter_set["Current function [A]"] = pybamm.Interpolant( self.dataset["Time [s]"], @@ -190,8 +190,8 @@ def rebuild( This method requires the self.build() method to be called first, and then rebuilds the model for a given parameter set. Specifically, - this method applies the given parameters, sets up the mesh and discretization if needed, and prepares the model - for simulations. + this method applies the given parameters, sets up the mesh and + discretisation if needed, and prepares the model for simulations. Parameters ---------- @@ -209,7 +209,7 @@ def rebuild( self.dataset = dataset self.parameters = parameters if parameters is not None: - self.set_parameter_classification(parameters) + self.classify_and_update_parameters(parameters) if init_soc is not None: self.set_init_soc(init_soc) @@ -227,29 +227,26 @@ def rebuild( # Clear solver and setup model self._solver._model_set_up = {} - def set_parameter_classification(self, parameters): + def classify_and_update_parameters(self, parameters): """ - Set the parameter classification for the model. + Update the parameter values according to their classification as either + 'matched_parameters' which require a model rebuild and + 'non_matched_parameters' which are standard inputs. Parameters ---------- - parameters : Pybop.ParameterSet - - Returns - ------- - None - The method updates attributes on self. + parameters : pybop.ParameterSet """ - processed_parameters = {param.name: param.value for param in parameters} + parameter_dictionary = {param.name: param.value for param in parameters} matched_parameters = { - param: processed_parameters[param] - for param in processed_parameters + param: parameter_dictionary[param] + for param in parameter_dictionary if param in self.rebuild_parameters } non_matched_parameters = { - param: processed_parameters[param] - for param in processed_parameters + param: parameter_dictionary[param] + for param in parameter_dictionary if param not in self.rebuild_parameters } @@ -261,9 +258,6 @@ def set_parameter_classification(self, parameters): self._unprocessed_parameter_set = self._parameter_set self.geometry = self.pybamm_model.default_geometry - if self.non_matched_parameters: - self.fit_keys = list(self.non_matched_parameters.keys()) - def reinit( self, inputs: Inputs, t: float = 0.0, x: Optional[np.ndarray] = None ) -> TimeSeriesState: @@ -305,7 +299,7 @@ def step(self, state: TimeSeriesState, time: np.ndarray) -> TimeSeriesState: state : TimeSeriesState The current state of the model time : np.ndarray - The time to predict the system to (in whatever time units the model is in) + The time to simulate the system until (in whatever time units the model is in) """ dt = time - state.t new_sol = self._solver.step( @@ -338,7 +332,7 @@ def simulate(self, inputs, t_eval) -> np.ndarray[np.float64]: if self._built_model is None: raise ValueError("Model must be built before calling simulate") else: - if not self.fit_keys and self.matched_parameters: + if self.matched_parameters and not self.non_matched_parameters: sol = self.solver.solve(self.built_model, t_eval=t_eval) else: @@ -355,9 +349,9 @@ def simulate(self, inputs, t_eval) -> np.ndarray[np.float64]: else: return [np.inf] - predictions = [sol[signal].data for signal in self.signal] + simulation = [sol[signal].data for signal in self.signal] - return np.vstack(predictions).T + return np.vstack(simulation).T def simulateS1(self, inputs, t_eval): """ @@ -386,6 +380,11 @@ def simulateS1(self, inputs, t_eval): if self._built_model is None: raise ValueError("Model must be built before calling simulate") else: + if self.matched_parameters: + raise ValueError( + "Cannot use sensitivies for parameters which require a model rebuild" + ) + if not isinstance(inputs, dict): inputs = {key: inputs[i] for i, key in enumerate(self.fit_keys)} @@ -400,7 +399,7 @@ def simulateS1(self, inputs, t_eval): calculate_sensitivities=True, ) - predictions = [sol[signal].data for signal in self.signal] + simulation = [sol[signal].data for signal in self.signal] sensitivities = [ np.array( @@ -409,7 +408,7 @@ def simulateS1(self, inputs, t_eval): for key in self.fit_keys ] - return np.vstack(predictions).T, np.dstack(sensitivities) + return np.vstack(simulation).T, np.dstack(sensitivities) else: return [np.inf], [np.inf] @@ -460,7 +459,7 @@ def predict( if PyBaMM models are not supported by the current simulation method. """ - parameter_set = parameter_set or self._parameter_set + parameter_set = parameter_set or self._unprocessed_parameter_set if inputs is not None: if not isinstance(inputs, dict): inputs = {key: inputs[i] for i, key in enumerate(self.fit_keys)} diff --git a/tests/integration/test_model_experiment_changes.py b/tests/integration/test_model_experiment_changes.py new file mode 100644 index 00000000..facac98e --- /dev/null +++ b/tests/integration/test_model_experiment_changes.py @@ -0,0 +1,103 @@ +import pybop +import pytest +import numpy as np + + +class TestModelAndExperimentChanges: + """ + A class to test that different inputs return different outputs. + """ + + @pytest.fixture( + params=[ + pybop.Parameter( # geometric parameter + "Negative particle radius [m]", + prior=pybop.Gaussian(6e-06, 0.1e-6), + bounds=[1e-6, 9e-6], + true_value=5.86e-6, + ), + pybop.Parameter( # non-geometric parameter + "Positive electrode diffusivity [m2.s-1]", + prior=pybop.Gaussian(3.43e-15, 1e-15), + bounds=[1e-15, 5e-15], + true_value=4e-15, + ), + ] + ) + def parameter(self, request): + return request.param + + @pytest.mark.integration + def test_changing_experiment(self, parameter): + # Change the experiment and check that the results are different. + + parameter_set = pybop.ParameterSet.pybamm("Chen2020") + parameters = [parameter] + init_soc = 0.5 + model = pybop.lithium_ion.SPM(parameter_set=parameter_set) + + t_eval = np.arange(0, 3600, 2) # Default 1C discharge to cut-off voltage + solution_1 = model.predict(init_soc=init_soc, t_eval=t_eval) + cost_1 = self.final_cost(solution_1, model, parameters, init_soc) + + experiment = pybop.Experiment(["Charge at 1C until 4.1 V (2 seconds period)"]) + solution_2 = model.predict( + init_soc=init_soc, experiment=experiment, inputs=[parameter.true_value] + ) + cost_2 = self.final_cost(solution_2, model, parameters, init_soc) + + with np.testing.assert_raises(AssertionError): + np.testing.assert_array_equal( + solution_1["Voltage [V]"].data, + solution_2["Voltage [V]"].data, + ) + + # The datasets are not corrupted so the costs should be zero + np.testing.assert_almost_equal(cost_1, 0) + np.testing.assert_almost_equal(cost_2, 0) + + @pytest.mark.integration + def test_changing_model(self, parameter): + # Change the model and check that the results are different. + + parameter_set = pybop.ParameterSet.pybamm("Chen2020") + parameters = [parameter] + init_soc = 0.5 + experiment = pybop.Experiment(["Charge at 1C until 4.1 V (2 seconds period)"]) + + model = pybop.lithium_ion.SPM(parameter_set=parameter_set) + solution_1 = model.predict(init_soc=init_soc, experiment=experiment) + cost_1 = self.final_cost(solution_1, model, parameters, init_soc) + + model = pybop.lithium_ion.SPMe(parameter_set=parameter_set) + solution_2 = model.predict(init_soc=init_soc, experiment=experiment) + cost_2 = self.final_cost(solution_2, model, parameters, init_soc) + + with np.testing.assert_raises(AssertionError): + np.testing.assert_array_equal( + solution_1["Voltage [V]"].data, + solution_2["Voltage [V]"].data, + ) + + # The datasets are not corrupted so the costs should be zero + np.testing.assert_almost_equal(cost_1, 0) + np.testing.assert_almost_equal(cost_2, 0) + + def final_cost(self, solution, model, parameters, init_soc): + # Compute the cost corresponding to a particular solution + x0 = np.array([parameters[0].true_value]) + dataset = pybop.Dataset( + { + "Time [s]": solution["Time [s]"].data, + "Current function [A]": solution["Current [A]"].data, + "Voltage [V]": solution["Voltage [V]"].data, + } + ) + signal = ["Voltage [V]"] + problem = pybop.FittingProblem( + model, parameters, dataset, signal=signal, x0=x0, init_soc=init_soc + ) + cost = pybop.RootMeanSquaredError(problem) + optim = pybop.Optimisation(cost, optimiser=pybop.PSO) + x, final_cost = optim.run() + return final_cost