Update default init_soc for DesignProblem (#422)

* Update default init_soc * Update CHANGELOG.md * Update check_params * Add pybamm_model as default attribute * Ensure predict uses unprocessed_model * Remove unused store_optimised_parameters * Update parameters.initial_value * Use any Initial SoC from parameter_set * Update spm_electrode_design.ipynb * Move Changelog entry * Fix merge mistake * Add tests * Update description * Update check for ECM init_soc --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
pybop-team · Aug 5, 2024 · 9e15f90 · 9e15f90
1 parent 8b09214
commit 9e15f90
Show file tree

Hide file tree

Showing 11 changed files with 122 additions and 107 deletions.
diff --git 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
diff --git 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,44 +505,52 @@ 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:
             return [np.inf]
 
     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
@@ -1,4 +1,5 @@
 from pybop.models.base_model import BaseModel, Inputs
+from pybop.parameters.parameter_set import ParameterSet
 
 
 class ECircuitModel(BaseModel):
@@ -46,22 +47,23 @@ 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":
                 print("Setting open-circuit voltage to default function")
                 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,21 +87,27 @@ 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.
 
         Parameters
         ----------
         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).
 
         Returns
         -------
         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
@@ -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
@@ -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
@@ -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
@@ -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)