Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creates a 'calc_esoh' property in the BaseModel #4825

Merged
merged 8 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Features

- Creates a 'calc_esoh' property in battery models ([#4825](https://github.com/pybamm-team/PyBaMM/pull/4825))
- Added 'get_summary_variables' to return dictionary of computed summary variables ([#4824](https://github.com/pybamm-team/PyBaMM/pull/4824))
- Added support for particle size distributions combined with particle mechanics. ([#4807](https://github.com/pybamm-team/PyBaMM/pull/4807))

Expand Down
8 changes: 8 additions & 0 deletions src/pybamm/models/base_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ def __init__(self, name="Unnamed model"):
self.is_discretised = False
self.y_slices = None

# Non-lithium ion models shouldn't calculate eSOH parameters
self._calc_esoh = False

@classmethod
def deserialise(cls, properties: dict):
"""
Expand Down Expand Up @@ -426,6 +429,11 @@ def input_parameters(self):
self._input_parameters = self._find_symbols(pybamm.InputParameter)
return self._input_parameters

@property
def calc_esoh(self):
"""Whether to include eSOH variables in the summary variables."""
return self._calc_esoh

def get_parameter_info(self, by_submodel=False):
"""
Extracts the parameter information and returns it as a dictionary.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ def __init__(self, options=None, name="Unnamed lithium-ion model", build=False):

self.set_standard_output_variables()

# Li models should calculate esoh by default
self._calc_esoh = True

def set_submodels(self, build):
self.set_external_circuit_submodel()
self.set_porosity_submodel()
Expand Down Expand Up @@ -93,6 +96,24 @@ def default_quick_plot_variables(self):
"Voltage [V]",
]

@property
def calc_esoh(self):
"""Whether to include eSOH variables in the summary variables."""
if (
self.options["particle phases"] not in ["1", ("1", "1")]
or self.options["working electrode"] != "both"
):
self._calc_esoh = False
return self._calc_esoh

@calc_esoh.setter
def calc_esoh(self, value: bool):
"""Set whether to include eSOH variables in the summary variables."""
if not isinstance(value, bool):
raise TypeError("`calc_esoh` arg needs to be a bool")

self._calc_esoh = value

def set_standard_output_variables(self):
super().set_standard_output_variables()

Expand Down
26 changes: 18 additions & 8 deletions src/pybamm/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ def solve(
t_eval=None,
solver=None,
save_at_cycles=None,
calc_esoh=True,
calc_esoh=None,
starting_solution=None,
initial_soc=None,
callbacks=None,
Expand Down Expand Up @@ -436,7 +436,8 @@ def solve(
calc_esoh : bool, optional
Whether to include eSOH variables in the summary variables. If `False`
then only summary variables that do not require the eSOH calculation
are calculated. Default is True.
are calculated.
If given, overwrites the default provided by the model.
starting_solution : :class:`pybamm.Solution`
The solution to start stepping from. If None (default), then self._solution
is used. Must be None if not using an experiment.
Expand Down Expand Up @@ -464,6 +465,20 @@ def solve(
if solver is None:
solver = self._solver

if calc_esoh is None:
calc_esoh = self._model.calc_esoh
else:
# stop 'True' override if model isn't suitable to calculate eSOH
if calc_esoh and not self._model.calc_esoh:
calc_esoh = False
warnings.warn(
UserWarning(
"Model is not suitable for calculating eSOH, "
"setting `calc_esoh` to False",
),
stacklevel=2,
)

callbacks = pybamm.callbacks.setup_callbacks(callbacks)
logs = {}

Expand Down Expand Up @@ -1021,12 +1036,7 @@ def step(
return self._solution

def _get_esoh_solver(self, calc_esoh):
if (
calc_esoh is False
or not isinstance(self._model, pybamm.lithium_ion.BaseModel)
or self._model.options["particle phases"] not in ["1", ("1", "1")]
or self._model.options["working electrode"] != "both"
):
if calc_esoh is False:
return None

return pybamm.lithium_ion.ElectrodeSOHSolver(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,11 @@ def test_insert_reference_electrode(self):
NotImplementedError, match="Reference electrode can only be"
):
model.insert_reference_electrode()

def test_setting_calc_esoh(self):
model = pybamm.lithium_ion.BaseModel()
model.calc_esoh = False
assert model.calc_esoh is False

with pytest.raises(TypeError, match="`calc_esoh` arg needs to be a bool"):
model.calc_esoh = "Yes"
12 changes: 12 additions & 0 deletions tests/unit/test_simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -678,3 +678,15 @@ def test_battery_model_with_input_height(self):
inputs = {"Electrode height [m]": 0.2}
sim = pybamm.Simulation(model=model, parameter_values=parameter_values)
sim.solve(t_eval=t_eval, inputs=inputs)

def test_simulation_cannot_force_calc_esoh(self):
model = pybamm.BaseModel()
v = pybamm.Variable("v")
model.rhs = {v: -v}
model.initial_conditions = {v: 1}
sim = pybamm.Simulation(model)

with pytest.warns(
UserWarning, match="Model is not suitable for calculating eSOH"
):
sim.solve([0, 1], calc_esoh=True)