diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c0005a6..f7e08058 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Features - [#364](https://github.com/pybop-team/PyBOP/pull/364) - Adds the MultiFittingProblem class and the multi_fitting example script. +- [#444](https://github.com/pybop-team/PyBOP/issues/444) - Merge `BaseModel` `build()` and `rebuild()` functionality. - [#435](https://github.com/pybop-team/PyBOP/pull/435) - Adds SLF001 linting for private members. - [#418](https://github.com/pybop-team/PyBOP/issues/418) - Wraps the `get_parameter_info` method from PyBaMM to get a dictionary of parameter names and types. - [#413](https://github.com/pybop-team/PyBOP/pull/413) - Adds `DesignCost` functionality to `WeightedCost` class with additional tests. diff --git a/examples/notebooks/LG_M50_ECM/1-single-pulse-circuit-model.ipynb b/examples/notebooks/LG_M50_ECM/1-single-pulse-circuit-model.ipynb index f62cd833..94624abe 100644 --- a/examples/notebooks/LG_M50_ECM/1-single-pulse-circuit-model.ipynb +++ b/examples/notebooks/LG_M50_ECM/1-single-pulse-circuit-model.ipynb @@ -1639,8 +1639,7 @@ "outputs": [], "source": [ "problem.set_target(dataset_two_pulse)\n", - "model.parameter_set[\"Initial SoC\"] = 0.8 - 0.0075\n", - "model.rebuild(dataset_two_pulse)" + "model.build(dataset=dataset_two_pulse, initial_state={\"Initial SoC\": 0.8 - 0.0075})" ] }, { diff --git a/examples/notebooks/multi_model_identification.ipynb b/examples/notebooks/multi_model_identification.ipynb index 43fc92e6..1b26b286 100644 --- a/examples/notebooks/multi_model_identification.ipynb +++ b/examples/notebooks/multi_model_identification.ipynb @@ -147,7 +147,7 @@ "metadata": {}, "outputs": [], "source": [ - "synth_model.build(dataset, initial_state=initial_state)\n", + "synth_model.build(dataset=dataset, initial_state=initial_state)\n", "synth_model.signal = [\"Voltage [V]\"]\n", "values = synth_model.simulate(t_eval=t_eval, inputs={})" ] diff --git a/pybop/models/base_model.py b/pybop/models/base_model.py index 73382d9b..44694249 100644 --- a/pybop/models/base_model.py +++ b/pybop/models/base_model.py @@ -80,14 +80,10 @@ def __init__( Additional Attributes --------------------- - parameters : pybop.Parameters + pybamm_model : pybamm.BaseModel, optional + An instance of a PyBaMM model. + parameters : pybop.Parameters, optional The input parameters. - output_variables : list[str], optional - A list of names of variables to include in the solution object. - rebuild_parameters : dict - A list of parameters which require the model to be rebuilt (default: {}). - standard_parameters : dict - A list of standard (i.e. not rebuild) parameters (default: {}). param_check_counter : int A counter for the number of parameter checks (default: 0). allow_infeasible_solutions : bool, optional @@ -106,21 +102,20 @@ def __init__( self.pybamm_model = None self.parameters = Parameters() - self.rebuild_parameters = {} - self.standard_parameters = {} self.param_check_counter = 0 self.allow_infeasible_solutions = True - self.current_function = None def build( self, - dataset: Optional[Dataset] = None, parameters: Union[Parameters, dict] = None, - check_model: bool = True, + inputs: Optional[Inputs] = None, initial_state: Optional[dict] = None, + dataset: Optional[Dataset] = None, + check_model: bool = True, ) -> None: """ - Construct the PyBaMM model if not already built, and set parameters. + Construct the PyBaMM model, if not already built or if there are changes to any + `rebuild_parameters` or the initial state. This method initializes the model components, applies the given parameters, sets up the mesh and discretisation if needed, and prepares the model @@ -128,23 +123,29 @@ def build( Parameters ---------- - dataset : pybamm.Dataset, optional - The dataset to be used in the model construction. parameters : pybop.Parameters or Dict, optional A pybop Parameters class or dictionary containing parameter values to apply to the model. - check_model : bool, optional - If True, the model will be checked for correctness after construction. + inputs : Inputs + The input parameters to be used when building the model. initial_state : dict, optional A valid initial state, e.g. the initial state of charge or open-circuit voltage. Defaults to None, indicating that the existing initial state of charge (for an ECM) or initial concentrations (for an EChem model) will be used. + dataset : pybop.Dataset or dict, optional + The dataset to be used in the model construction. + check_model : bool, optional + If True, the model will be checked for correctness after construction. """ - if parameters is not None: - self.parameters = parameters - self.classify_and_update_parameters(self.parameters) + if parameters is not None or inputs is not None: + # Classify parameters and clear the model if rebuild required + inputs = self.classify_parameters(parameters, inputs=inputs) if initial_state is not None: - self.set_initial_state(initial_state) + # Clear the model if rebuild required (currently if any initial state) + self.set_initial_state(initial_state, inputs=inputs) + + if dataset is not None: + self.set_current_function(dataset) if self._built_model: return @@ -156,8 +157,8 @@ def build( else: if not self.pybamm_model._built: # noqa: SLF001 self.pybamm_model.build_model() - self.set_params(dataset=dataset) + self.set_parameters() self._mesh = pybamm.Mesh(self.geometry, self.submesh_types, self.var_pts) self._disc = pybamm.Discretisation( mesh=self.mesh, @@ -203,7 +204,7 @@ def convert_to_pybamm_initial_state(self, initial_state: dict): else: raise ValueError(f'Unrecognised initial state: "{list(initial_state)[0]}"') - def set_initial_state(self, initial_state: dict): + def set_initial_state(self, initial_state: dict, inputs: Optional[Inputs] = None): """ Set the initial state of charge or concentrations for the battery model. @@ -211,20 +212,27 @@ def set_initial_state(self, initial_state: dict): ---------- initial_state : dict A valid initial state, e.g. the initial state of charge or open-circuit voltage. + inputs : Inputs + The input parameters to be used when building the model. """ + self.clear() + initial_state = self.convert_to_pybamm_initial_state(initial_state) if isinstance(self.pybamm_model, pybamm.equivalent_circuit.Thevenin): - initial_state = self.get_initial_state(initial_state) + initial_state = self.get_initial_state(initial_state, inputs=inputs) self._unprocessed_parameter_set.update({"Initial SoC": initial_state}) else: - # Temporary construction of attribute for PyBaMM + if not self.pybamm_model._built: # noqa: SLF001 + self.pybamm_model.build_model() + + # Temporary construction of attributes for PyBaMM self.model = self._model = self.pybamm_model self._unprocessed_parameter_values = self._unprocessed_parameter_set - # Set initial SOC via PyBaMM's Simulation class - pybamm.Simulation.set_initial_soc(self, initial_state, inputs=None) + # Set initial state via PyBaMM's Simulation class + pybamm.Simulation.set_initial_soc(self, initial_state, inputs=inputs) # Update the default parameter set for consistency self._unprocessed_parameter_set = self._parameter_values @@ -238,37 +246,36 @@ def set_initial_state(self, initial_state: dict): # Use a copy of the updated default parameter set self._parameter_set = self._unprocessed_parameter_set.copy() - def set_params(self, rebuild: bool = False, dataset: Dataset = None): + def set_current_function(self, dataset: Union[Dataset, dict]): + """ + Update the input current function according to the data. + + Parameters + ---------- + dataset : pybop.Dataset or dict, optional + The dataset to be used in the model construction. + """ + if "Current function [A]" in self._parameter_set.keys(): + self._parameter_set["Current function [A]"] = pybamm.Interpolant( + dataset["Time [s]"], + dataset["Current function [A]"], + pybamm.t, + ) + + def set_parameters(self): """ Assign the parameters to the model. This method processes the model with the given parameters, sets up the geometry, and updates the model instance. """ - if self.model_with_set_params and not rebuild: + if self._model_with_set_params: return - # Mark any simulation inputs in the parameter set - for key in self.standard_parameters.keys(): - self._parameter_set[key] = "[input]" - - if "Current function [A]" in self._parameter_set.keys(): - if dataset is not None and (not self.rebuild_parameters or not rebuild): - if "Current function [A]" not in self.parameters.keys(): - self.current_function = pybamm.Interpolant( - dataset["Time [s]"], - dataset["Current function [A]"], - pybamm.t, - ) - self._parameter_set["Current function [A]"] = self.current_function - elif rebuild and self.current_function is not None: - self._parameter_set["Current function [A]"] = self.current_function - self._model_with_set_params = self._parameter_set.process_model( self._unprocessed_model, inplace=False ) - if self.geometry is not None: - self._parameter_set.process_geometry(self.geometry) + self._parameter_set.process_geometry(self._geometry) self.pybamm_model = self._model_with_set_params def clear(self): @@ -281,71 +288,26 @@ def clear(self): self._mesh = None self._disc = None - def rebuild( - self, - dataset: Optional[Dataset] = None, - parameters: Union[Parameters, dict] = None, - check_model: bool = True, - initial_state: Optional[dict] = None, - ) -> None: - """ - Rebuild the PyBaMM model for a given set of inputs. - - 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 - discretisation if needed, and prepares the model for simulations. - - Parameters - ---------- - dataset : pybamm.Dataset, optional - The dataset to be used in the model construction. - parameters : pybop.Parameters or Dict, optional - A pybop Parameters class or dictionary containing parameter values to apply to the model. - check_model : bool, optional - If True, the model will be checked for correctness after construction. - initial_state : dict, optional - A valid initial state, e.g. the initial state of charge or open-circuit voltage. - Defaults to None, indicating that the existing initial state of charge (for an ECM) - or initial concentrations (for an EChem model) will be used. - """ - if parameters is not None: - self.classify_and_update_parameters(parameters) - - if initial_state is not None: - self.set_initial_state(initial_state) - - if self._built_model is None: - raise ValueError("Model must be built before calling rebuild") - - self.set_params(rebuild=True, dataset=dataset) - self._mesh = pybamm.Mesh(self.geometry, self.submesh_types, self.var_pts) - self._disc = pybamm.Discretisation( - mesh=self.mesh, - spatial_methods=self.spatial_methods, - check_model=check_model, - ) - self._built_model = self._disc.process_model( - self._model_with_set_params, inplace=False - ) - - # Clear solver and setup model - self._solver._model_set_up = {} # noqa: SLF001 - - def classify_and_update_parameters(self, parameters: Parameters): + def classify_parameters( + self, parameters: Optional[Parameters] = None, inputs: Optional[Inputs] = None + ): """ - Update the parameter values according to their classification as either - 'rebuild_parameters' which require a model rebuild and - 'standard_parameters' which do not. + Check for any 'rebuild_parameters' which require a model rebuild and + update the unprocessed_parameter_set if a rebuild is required. Parameters ---------- - parameters : pybop.Parameters - The input parameters. + parameters : Parameters, optional + The optimisation parameters. Defaults to None, meaning that the parameters + attribute is not modified. + inputs : Inputs, optional + The input parameters for the simulation (default: None). """ - self.parameters = parameters or Parameters() + self.parameters = parameters or self.parameters + # Compile all parameters and inputs parameter_dictionary = self.parameters.as_dict() + parameter_dictionary.update(inputs or {}) rebuild_parameters = { param: parameter_dictionary[param] @@ -358,16 +320,25 @@ def classify_and_update_parameters(self, parameters: Parameters): if param not in self.geometric_parameters } - self.rebuild_parameters.update(rebuild_parameters) - self.standard_parameters.update(standard_parameters) + # Mark any standard parameters in the active parameter set and pass as inputs + for key in standard_parameters.keys(): + self._parameter_set[key] = "[input]" - if self.rebuild_parameters: - self._geometry = self.pybamm_model.default_geometry + # Clear any built model, update the parameter set and geometry if rebuild required + if rebuild_parameters: + requires_rebuild = False + # A rebuild is required if any of the rebuild parameter values have changed + for key, value in rebuild_parameters.items(): + if value != self._unprocessed_parameter_set[key]: + requires_rebuild = True + if requires_rebuild: + self.clear() + self._geometry = self.pybamm_model.default_geometry + # Update both the active and unprocessed parameter sets for consistency + self._parameter_set.update(rebuild_parameters) + self._unprocessed_parameter_set.update(rebuild_parameters) - # Update both the active and unprocessed parameter sets for consistency - if self._parameter_set is not None: - self._parameter_set.update(parameter_dictionary) - self._unprocessed_parameter_set.update(parameter_dictionary) + return standard_parameters def reinit( self, inputs: Inputs, t: float = 0.0, x: Optional[np.ndarray] = None @@ -445,22 +416,8 @@ def simulate( """ inputs = self.parameters.verify(inputs) - if self._built_model is None: - raise ValueError("Model must be built before calling simulate") - - requires_rebuild = False - # A rebuild is required if any of the rebuild parameter values have changed - for key, value in inputs.items(): - if key in self.rebuild_parameters: - if value != self.parameters[key].value: - requires_rebuild = True - # Or if the simulation is set to start from a specific initial value - if initial_state is not None: - requires_rebuild = True - - if requires_rebuild: - self.parameters.update(values=list(inputs.values())) - self.rebuild(parameters=self.parameters, initial_state=initial_state) + # Build or rebuild if required + self.build(inputs=inputs, initial_state=initial_state) if not self.check_params( inputs=inputs, @@ -501,14 +458,16 @@ def simulateS1( """ inputs = self.parameters.verify(inputs) - if self._built_model is None: - raise ValueError("Model must be built before calling simulate") - - if self.rebuild_parameters or initial_state is not None: + if initial_state is not None or any( + key in self.geometric_parameters for key in inputs.keys() + ): raise ValueError( "Cannot use sensitivies for parameters which require a model rebuild" ) + # Build if required + self.build(inputs=inputs, initial_state=initial_state) + if not self.check_params( inputs=inputs, allow_infeasible_solutions=self.allow_infeasible_solutions, diff --git a/pybop/models/empirical/base_ecm.py b/pybop/models/empirical/base_ecm.py index 739c64eb..d329cf38 100644 --- a/pybop/models/empirical/base_ecm.py +++ b/pybop/models/empirical/base_ecm.py @@ -158,8 +158,8 @@ def get_initial_state( if isinstance(initial_value, str) and initial_value.endswith("V"): V_init = float(initial_value[:-1]) - V_min = parameter_values.evaluate(param.voltage_low_cut) - V_max = parameter_values.evaluate(param.voltage_high_cut) + V_min = parameter_values.evaluate(param.voltage_low_cut, inputs=inputs) + V_max = parameter_values.evaluate(param.voltage_high_cut, inputs=inputs) if not V_min <= V_init <= V_max: raise ValueError( diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 4fda777d..55cd2ef8 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -60,18 +60,6 @@ def model(self, request): model = request.param return model.copy() - @pytest.mark.unit - def test_simulate_without_build_model(self, model): - with pytest.raises( - ValueError, match="Model must be built before calling simulate" - ): - model.simulate(None, None) - - with pytest.raises( - ValueError, match="Model must be built before calling simulate" - ): - model.simulateS1(None, None) - @pytest.mark.unit def test_non_default_solver(self): solver = pybamm.CasadiSolver( @@ -159,21 +147,15 @@ def test_build(self, model): @pytest.mark.unit def test_rebuild(self, model): - # Test rebuild before build - with pytest.raises( - ValueError, match="Model must be built before calling rebuild" - ): - model.rebuild() - model.build() initial_built_model = model._built_model assert model._built_model is not None - model.set_params() + model.set_parameters() assert model.model_with_set_params is not None # Test that the model can be built again - model.rebuild() + model.build() rebuilt_model = model._built_model assert rebuilt_model is not None @@ -251,7 +233,7 @@ def test_rebuild_geometric_parameters(self): # Test that the model can be rebuilt with different geometric parameters parameters["Positive particle radius [m]"].update(5e-06) parameters["Negative electrode thickness [m]"].update(45e-06) - model.rebuild(parameters=parameters) + model.build(parameters=parameters) rebuilt_model = model assert rebuilt_model._built_model is not None @@ -299,7 +281,7 @@ def test_reinit(self): state = model.reinit(inputs={}) np.testing.assert_array_almost_equal(state.as_ndarray(), np.array([[y0]])) - model.classify_and_update_parameters(pybop.Parameters(pybop.Parameter("y0"))) + model.classify_parameters(pybop.Parameters(pybop.Parameter("y0"))) state = model.reinit(inputs=[1]) np.testing.assert_array_almost_equal(state.as_ndarray(), np.array([[y0]])) @@ -339,7 +321,7 @@ def test_basemodel(self): with pytest.raises(NotImplementedError): base.approximate_capacity(x) - base.classify_and_update_parameters(parameters=None) + base.classify_parameters(parameters=None) assert isinstance(base.parameters, pybop.Parameters) @pytest.mark.unit