Creates a 'calc_esoh' property in the BaseModel (#4825)

* Creates a 'calc_esoh' property in the BaseModel, allows Simulation to override
False in BaseModel, True in lithium_ion.BaseModel

* Fix style issue

* Update changelog

* Add setter for calc_esoh attr in lithium BaseModel

* Apply suggestions from code review

Make docstrings triple quotes

Co-authored-by: Eric G. Kratz <[email protected]>

---------

Co-authored-by: Eric G. Kratz <[email protected]>

CHANGELOG.md

@@ -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))
 

src/pybamm/models/base_model.py

@@ -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):
         """
@@ -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.

src/pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py

@@ -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()
@@ -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()
 

src/pybamm/simulation.py

@@ -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,
@@ -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.
@@ -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 = {}
 
@@ -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(

tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_base_lithium_ion_model.py

@@ -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"

tests/unit/test_simulation.py

@@ -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)