From 9e15f90b75c4f5fb04cb17c0807dd0b5e1a2e6fb Mon Sep 17 00:00:00 2001
From: NicolaCourtier <45851982+NicolaCourtier@users.noreply.github.com>
Date: Mon, 5 Aug 2024 12:11:05 +0100
Subject: [PATCH] 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>
---
CHANGELOG.md | 3 +-
examples/notebooks/spm_electrode_design.ipynb | 4 +-
pybop/models/base_model.py | 79 ++++++++++++-------
pybop/models/empirical/base_ecm.py | 18 +++--
pybop/models/empirical/ecm.py | 20 -----
pybop/models/lithium_ion/base_echem.py | 10 ++-
pybop/optimisers/base_optimiser.py | 14 ----
pybop/parameters/parameter.py | 24 +++---
pybop/problems/design_problem.py | 30 +++----
tests/unit/test_models.py | 20 ++++-
tests/unit/test_problem.py | 7 ++
11 files changed, 122 insertions(+), 107 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 863d4028e..2b6d4f96a 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 e7b2fd57d..6bc92a122 100644
--- a/examples/notebooks/spm_electrode_design.ipynb
+++ b/examples/notebooks/spm_electrode_design.ipynb
@@ -267,7 +267,7 @@
{
"data": {
"image/svg+xml": [
- ""
+ ""
]
},
"metadata": {},
@@ -306,7 +306,7 @@
{
"data": {
"image/svg+xml": [
- ""
+ ""
]
},
"metadata": {},
diff --git a/pybop/models/base_model.py b/pybop/models/base_model.py
index 9e547b656..bdb671892 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 38d94d147..82ed3020a 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 893b30bc6..f831aee83 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 f60f500d3..71f803c04 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 c24b081dd..c6d68e814 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 cb1a19369..07eca7db0 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 74b8ae715..e6a828665 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 551379b71..36d184a02 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 c2c40a038..b7660cfc1 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