diff --git a/CHANGELOG.md b/CHANGELOG.md index 863d4028..2b6d4f96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ ## Bug Fixes +- [#421](https://github.com/pybop-team/PyBOP/issues/421) - Adds a default value for the initial SOC for design problems. + ## Breaking Changes # [v24.6.1](https://github.com/pybop-team/PyBOP/tree/v24.6.1) - 2024-07-31 @@ -22,7 +24,6 @@ ## Bug Fixes - ## Breaking Changes # [v24.6](https://github.com/pybop-team/PyBOP/tree/v24.6) - 2024-07-08 diff --git a/examples/notebooks/spm_electrode_design.ipynb b/examples/notebooks/spm_electrode_design.ipynb index e7b2fd57..6bc92a12 100644 --- a/examples/notebooks/spm_electrode_design.ipynb +++ b/examples/notebooks/spm_electrode_design.ipynb @@ -267,7 +267,7 @@ { "data": { "image/svg+xml": [ - "01000200030002.42.62.833.23.43.63.84ReferenceOptimisedOptimised ComparisonTime / sVoltage / V" + "01000200030002.42.62.833.23.43.63.84ReferenceOptimisedOptimised ComparisonTime / sVoltage / V" ] }, "metadata": {}, @@ -306,7 +306,7 @@ { "data": { "image/svg+xml": [ - "70μ80μ90μ100μ320340360380400Cost LandscapePositive electrode thickness [m]Positive particle radius [m]" + "70μ80μ90μ100μ320340360380400Cost LandscapePositive electrode thickness [m]Positive particle radius [m]" ] }, "metadata": {}, diff --git a/pybop/models/base_model.py b/pybop/models/base_model.py index 9e547b65..bdb67189 100644 --- a/pybop/models/base_model.py +++ b/pybop/models/base_model.py @@ -64,6 +64,7 @@ def __init__(self, name="Base Model", parameter_set=None): else: # a pybop parameter set self._parameter_set = pybamm.ParameterValues(parameter_set.params) + self.pybamm_model = None self.parameters = Parameters() self.dataset = None self.signal = None @@ -215,7 +216,7 @@ def rebuild( parameters : pybop.Parameters or Dict, optional A pybop Parameters class or dictionary containing parameter values to apply to the model. parameter_set : pybop.parameter_set, optional - A PyBOP parameter set object or a dictionary containing the parameter values + A PyBOP parameter set object or a dictionary containing the parameter values. check_model : bool, optional If True, the model will be checked for correctness after construction. init_soc : float, optional @@ -461,18 +462,18 @@ def simulateS1(self, inputs: Inputs, t_eval: np.array): def predict( self, - inputs: Inputs = None, - t_eval: np.array = None, - parameter_set: ParameterSet = None, - experiment: Experiment = None, + inputs: Optional[Inputs] = None, + t_eval: Optional[np.array] = None, + parameter_set: Optional[ParameterSet] = None, + experiment: Optional[Experiment] = None, init_soc: Optional[float] = None, ) -> dict[str, np.ndarray[np.float64]]: """ Solve the model using PyBaMM's simulation framework and return the solution. This method sets up a PyBaMM simulation by configuring the model, parameters, experiment - (if any), and initial state of charge (if provided). It then solves the simulation and - returns the resulting solution object. + or time vector, and initial state of charge (if provided). Either 't_eval' or 'experiment' + must be provided. It then solves the simulation and returns the resulting solution object. Parameters ---------- @@ -504,35 +505,43 @@ def predict( if PyBaMM models are not supported by the current simulation method. """ - inputs = self.parameters.verify(inputs) - - if not self.pybamm_model._built: - self.pybamm_model.build_model() + if self._unprocessed_model is None: + raise ValueError( + "The predict method currently only supports PyBaMM models." + ) + elif not self._unprocessed_model._built: + self._unprocessed_model.build_model() parameter_set = parameter_set or self._unprocessed_parameter_set if inputs is not None: + inputs = self.parameters.verify(inputs) parameter_set.update(inputs) + if init_soc is not None and isinstance( + self.pybamm_model, pybamm.equivalent_circuit.Thevenin + ): + parameter_set["Initial SoC"] = init_soc + init_soc = None + if self.check_params( - inputs=inputs, parameter_set=parameter_set, allow_infeasible_solutions=self.allow_infeasible_solutions, ): - if self._unprocessed_model is not None: - if experiment is None: - return pybamm.Simulation( - self._unprocessed_model, - parameter_values=parameter_set, - ).solve(t_eval=t_eval, initial_soc=init_soc) - else: - return pybamm.Simulation( - self._unprocessed_model, - experiment=experiment, - parameter_values=parameter_set, - ).solve(initial_soc=init_soc) + if experiment is not None: + return pybamm.Simulation( + model=self._unprocessed_model, + experiment=experiment, + parameter_values=parameter_set, + ).solve(initial_soc=init_soc) + elif t_eval is not None: + return pybamm.Simulation( + model=self._unprocessed_model, + parameter_values=parameter_set, + ).solve(t_eval=t_eval, initial_soc=init_soc) else: raise ValueError( - "This sim method currently only supports PyBaMM models" + "The predict method requires either an experiment or " + "t_eval to be specified." ) else: @@ -540,8 +549,8 @@ def predict( def check_params( self, - inputs: Inputs = None, - parameter_set: ParameterSet = None, + inputs: Optional[Inputs] = None, + parameter_set: Optional[ParameterSet] = None, allow_infeasible_solutions: bool = True, ): """ @@ -551,6 +560,8 @@ def check_params( ---------- inputs : Inputs The input parameters for the simulation. + parameter_set : pybop.parameter_set, optional + A PyBOP parameter set object or a dictionary containing the parameter values. allow_infeasible_solutions : bool, optional If True, infeasible parameter values will be allowed in the optimisation (default: True). @@ -560,14 +571,20 @@ def check_params( A boolean which signifies whether the parameters are compatible. """ - inputs = self.parameters.verify(inputs) + inputs = self.parameters.verify(inputs) or {} + parameter_set = parameter_set or self._parameter_set return self._check_params( - inputs=inputs, allow_infeasible_solutions=allow_infeasible_solutions + inputs=inputs, + parameter_set=parameter_set, + allow_infeasible_solutions=allow_infeasible_solutions, ) def _check_params( - self, inputs: Inputs = None, allow_infeasible_solutions: bool = True + self, + inputs: Inputs, + parameter_set: ParameterSet, + allow_infeasible_solutions: bool = True, ): """ A compatibility check for the model parameters which can be implemented by subclasses @@ -577,6 +594,8 @@ def _check_params( ---------- inputs : Inputs The input parameters for the simulation. + parameter_set : pybop.parameter_set + A PyBOP parameter set object or a dictionary containing the parameter values. allow_infeasible_solutions : bool, optional If True, infeasible parameter values will be allowed in the optimisation (default: True). diff --git a/pybop/models/empirical/base_ecm.py b/pybop/models/empirical/base_ecm.py index 38d94d14..82ed3020 100644 --- a/pybop/models/empirical/base_ecm.py +++ b/pybop/models/empirical/base_ecm.py @@ -1,4 +1,5 @@ from pybop.models.base_model import BaseModel, Inputs +from pybop.parameters.parameter_set import ParameterSet class ECircuitModel(BaseModel): @@ -46,15 +47,14 @@ def __init__( model_options = dict(build=False) for key, value in model_kwargs.items(): model_options[key] = value - self.pybamm_model = pybamm_model(**model_options) - self._unprocessed_model = self.pybamm_model + pybamm_model = pybamm_model(**model_options) # Correct OCP if set to default if ( parameter_set is not None and "Open-circuit voltage [V]" in parameter_set.keys() ): - default_ocp = self.pybamm_model.default_parameter_values[ + default_ocp = pybamm_model.default_parameter_values[ "Open-circuit voltage [V]" ] if parameter_set["Open-circuit voltage [V]"] == "default": @@ -62,6 +62,8 @@ def __init__( parameter_set["Open-circuit voltage [V]"] = default_ocp super().__init__(name=name, parameter_set=parameter_set) + self.pybamm_model = pybamm_model + self._unprocessed_model = self.pybamm_model # Set parameters, using either the provided ones or the default self.default_parameter_values = self.pybamm_model.default_parameter_values @@ -85,7 +87,12 @@ def __init__( self._disc = None self.geometric_parameters = {} - def _check_params(self, inputs: Inputs = None, allow_infeasible_solutions=True): + def _check_params( + self, + inputs: Inputs, + parameter_set: ParameterSet, + allow_infeasible_solutions: bool = True, + ): """ Check the compatibility of the model parameters. @@ -93,6 +100,8 @@ def _check_params(self, inputs: Inputs = None, allow_infeasible_solutions=True): ---------- inputs : Inputs The input parameters for the simulation. + parameter_set : pybop.parameter_set + A PyBOP parameter set object or a dictionary containing the parameter values. allow_infeasible_solutions : bool, optional If True, infeasible parameter values will be allowed in the optimisation (default: True). @@ -100,6 +109,5 @@ def _check_params(self, inputs: Inputs = None, allow_infeasible_solutions=True): ------- bool A boolean which signifies whether the parameters are compatible. - """ return True diff --git a/pybop/models/empirical/ecm.py b/pybop/models/empirical/ecm.py index 893b30bc..f831aee8 100644 --- a/pybop/models/empirical/ecm.py +++ b/pybop/models/empirical/ecm.py @@ -1,7 +1,6 @@ from pybamm import equivalent_circuit as pybamm_equivalent_circuit from pybop.models.empirical.base_ecm import ECircuitModel -from pybop.parameters.parameter import Inputs class Thevenin(ECircuitModel): @@ -44,22 +43,3 @@ def __init__( super().__init__( pybamm_model=pybamm_equivalent_circuit.Thevenin, name=name, **model_kwargs ) - - def _check_params(self, inputs: Inputs = None, allow_infeasible_solutions=True): - """ - Check the compatibility of the model parameters. - - Parameters - ---------- - inputs : Inputs - The input parameters for the simulation. - allow_infeasible_solutions : bool, optional - If True, infeasible parameter values will be allowed in the optimisation (default: True). - - Returns - ------- - bool - A boolean which signifies whether the parameters are compatible. - - """ - return True diff --git a/pybop/models/lithium_ion/base_echem.py b/pybop/models/lithium_ion/base_echem.py index f60f500d..71f803c0 100644 --- a/pybop/models/lithium_ion/base_echem.py +++ b/pybop/models/lithium_ion/base_echem.py @@ -3,6 +3,7 @@ from pybamm import lithium_ion as pybamm_lithium_ion from pybop.models.base_model import BaseModel, Inputs +from pybop.parameters.parameter_set import ParameterSet class EChemBaseModel(BaseModel): @@ -85,7 +86,10 @@ def __init__( self.geometric_parameters = self.set_geometric_parameters() def _check_params( - self, inputs: Inputs = None, parameter_set=None, allow_infeasible_solutions=True + self, + inputs: Inputs, + parameter_set: ParameterSet, + allow_infeasible_solutions: bool = True, ): """ Check compatibility of the model parameters. @@ -94,6 +98,8 @@ def _check_params( ---------- inputs : Inputs The input parameters for the simulation. + parameter_set : pybop.parameter_set + A PyBOP parameter set object or a dictionary containing the parameter values. allow_infeasible_solutions : bool, optional If True, infeasible parameter values will be allowed in the optimisation (default: True). @@ -102,8 +108,6 @@ def _check_params( bool A boolean which signifies whether the parameters are compatible. """ - parameter_set = parameter_set or self._parameter_set - if self.pybamm_model.options["working electrode"] == "positive": electrode_params = [ ( diff --git a/pybop/optimisers/base_optimiser.py b/pybop/optimisers/base_optimiser.py index c24b081d..c6d68e81 100644 --- a/pybop/optimisers/base_optimiser.py +++ b/pybop/optimisers/base_optimiser.py @@ -199,20 +199,6 @@ def _run(self): """ raise NotImplementedError - def store_optimised_parameters(self, x): - """ - Update the problem parameters with optimised values. - - The optimised parameter values are stored within the associated PyBOP parameter class. - - Parameters - ---------- - x : array-like - Optimised parameter values. - """ - for i, param in enumerate(self.cost.parameters): - param.update(value=x[i]) - def check_optimal_parameters(self, x): """ Check if the optimised parameters are physically viable. diff --git a/pybop/parameters/parameter.py b/pybop/parameters/parameter.py index cb1a1936..07eca7db 100644 --- a/pybop/parameters/parameter.py +++ b/pybop/parameters/parameter.py @@ -189,8 +189,15 @@ def get_initial_value(self) -> float: Return the initial value of each parameter. """ if self.initial_value is None: - sample = self.rvs(1) - self.update(initial_value=sample[0]) + if self.prior is not None: + sample = self.rvs(1)[0] + self.update(initial_value=sample) + else: + warnings.warn( + "Initial value or Prior are None, proceeding without initial value.", + UserWarning, + stacklevel=2, + ) return self.initial_value @@ -409,17 +416,8 @@ def initial_value(self) -> np.ndarray: initial_values = [] for param in self.param.values(): - if param.initial_value is None: - if param.prior is not None: - initial_value = param.rvs(1)[0] - param.update(initial_value=initial_value) - else: - warnings.warn( - "Initial value or Prior are None, proceeding without initial value.", - UserWarning, - stacklevel=2, - ) - initial_values.append(param.initial_value) + initial_value = param.get_initial_value() + initial_values.append(initial_value) return np.asarray(initial_values) diff --git a/pybop/problems/design_problem.py b/pybop/problems/design_problem.py index 74b8ae71..e6a82866 100644 --- a/pybop/problems/design_problem.py +++ b/pybop/problems/design_problem.py @@ -27,7 +27,7 @@ class DesignProblem(BaseProblem): additional_variables : List[str], optional Additional variables to observe and store in the solution (default additions are: ["Time [s]", "Current [A]"]). init_soc : float, optional - Initial state of charge (default: None). + Initial state of charge (default: 1.0). """ def __init__( @@ -47,10 +47,13 @@ def __init__( signal = ["Voltage [V]"] additional_variables.extend(["Time [s]", "Current [A]"]) additional_variables = list(set(additional_variables)) - self.warning_patterns = [ - "Ah is greater than", - "Non-physical point encountered", - ] + + if init_soc is None: + if "Initial SoC" in model._parameter_set.keys(): + init_soc = model._parameter_set["Initial SoC"] + else: + init_soc = 1.0 # default value + super().__init__( parameters, model, @@ -61,24 +64,15 @@ def __init__( ) self.experiment = experiment - # Build the model if required - if experiment is not None: - # Leave the build until later to apply the experiment - self._model.classify_and_update_parameters(self.parameters) - - elif self._model._built_model is None: - self._model.build( - experiment=self.experiment, - parameters=self.parameters, - check_model=self.check_model, - init_soc=self.init_soc, - ) - # Add an example dataset for plotting comparison sol = self.evaluate(self.parameters.as_dict("initial")) self._time_data = sol["Time [s]"] self._target = {key: sol[key] for key in self.signal} self._dataset = None + self.warning_patterns = [ + "Ah is greater than", + "Non-physical point encountered", + ] def evaluate(self, inputs: Inputs, update_capacity=False): """ diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 551379b7..36d184a0 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -88,7 +88,10 @@ def test_non_default_solver(self): def test_predict_without_pybamm(self, model): model._unprocessed_model = None - with pytest.raises(ValueError): + with pytest.raises( + ValueError, + match="The predict method currently only supports PyBaMM models.", + ): model.predict(None, None) @pytest.mark.unit @@ -116,6 +119,12 @@ def test_predict_with_inputs(self, model): res = model.predict(t_eval=t_eval, inputs=inputs) assert len(res["Voltage [V]"].data) == 100 + with pytest.raises( + ValueError, + match="The predict method requires either an experiment or t_eval to be specified.", + ): + model.predict(inputs=inputs) + @pytest.mark.unit def test_predict_without_allow_infeasible_solutions(self, model): if isinstance(model, (pybop.lithium_ion.SPM, pybop.lithium_ion.SPMe)): @@ -223,6 +232,12 @@ def test_rebuild_geometric_parameters(self): t_eval = np.linspace(0, 100, 100) out_init = initial_built_model.predict(t_eval=t_eval) + with pytest.raises( + ValueError, + match="Cannot use sensitivies for parameters which require a model rebuild", + ): + model.simulateS1(t_eval=t_eval, inputs=parameters.as_dict()) + # 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) @@ -332,6 +347,9 @@ def test_thevenin_model(self): == model.pybamm_model.default_parameter_values["Open-circuit voltage [V]"] ) + model.predict(init_soc=0.5, t_eval=np.arange(0, 10, 5)) + assert model._parameter_set["Initial SoC"] == 0.5 + @pytest.mark.unit def test_check_params(self): base = pybop.BaseModel() diff --git a/tests/unit/test_problem.py b/tests/unit/test_problem.py index c2c40a03..b7660cfc 100644 --- a/tests/unit/test_problem.py +++ b/tests/unit/test_problem.py @@ -171,11 +171,18 @@ def test_design_problem(self, parameters, experiment, model): assert ( problem._model._built_model is None ) # building postponed with input experiment + assert problem.init_soc == 1.0 # Test model.predict model.predict(inputs=[1e-5, 1e-5], experiment=experiment) model.predict(inputs=[3e-5, 3e-5], experiment=experiment) + # Test init_soc from parameter_set + model = pybop.empirical.Thevenin() + model._parameter_set["Initial SoC"] = 0.8 + problem = pybop.DesignProblem(model, pybop.Parameters(), experiment) + assert problem.init_soc == 0.8 + @pytest.mark.unit def test_problem_construct_with_model_predict( self, parameters, model, dataset, signal