Merge pull request #1760 from pybamm-team/issue-1726-sumvar

Issue 1726 sumvar
pybamm-team · Oct 27, 2021 · fab46fa · fab46fa
2 parents 8266671 + 4bab6a5
commit fab46fa
Show file tree

Hide file tree

Showing 6 changed files with 133 additions and 50 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,10 +2,18 @@
 
 ## Features
 
+-   Summary variables can now be user-determined ([#1759](https://github.com/pybamm-team/PyBaMM/pull/1759))
 -   Added `all_first_states` to the `Solution` object for a simulation with experiment ([#1759](https://github.com/pybamm-team/PyBaMM/pull/1759))
 -   Added a new method (`create_gif`) in `QuickPlot`, `Simulation` and `BatchStudy` to create a GIF of a simulation ([#1754](https://github.com/pybamm-team/PyBaMM/pull/1754))
+-   Added more examples for the `BatchStudy` class ([#1747](https://github.com/pybamm-team/PyBaMM/pull/1747))
 -   SEI models can now be included in the half-cell model ([#1705](https://github.com/pybamm-team/PyBaMM/pull/1705))
 
+## Bug fixes
+
+-   Half-cell model and lead-acid models can now be simulated with `Experiment`s ([#1759](https://github.com/pybamm-team/PyBaMM/pull/1759))
+-   Removed in-place modification of the solution objects by `QuickPlot` ([#1747](https://github.com/pybamm-team/PyBaMM/pull/1747))
+-   Fixed vector-vector multiplication bug that was causing errors in the SPM with constant voltage or power ([#1735](https://github.com/pybamm-team/PyBaMM/pull/1735))
+
 # [v21.9](https://github.com/pybamm-team/PyBaMM/tree/v21.9) - 2021-09-30
 
 ## Features

diff --git a/pybamm/models/full_battery_models/base_battery_model.py b/pybamm/models/full_battery_models/base_battery_model.py
@@ -717,6 +717,7 @@ def build_model(self):
 
         pybamm.logger.debug("Setting degradation variables ({})".format(self.name))
         self.set_degradation_variables()
+        self.set_summary_variables()
 
         # Massive hack for consistent delta_phi = phi_s - phi_e with SPMe
         # This needs to be corrected
@@ -741,6 +742,30 @@ def new_empty_copy(self):
         new_model.length_scales = self.length_scales
         return new_model
 
+    @property
+    def summary_variables(self):
+        return self._summary_variables
+
+    @summary_variables.setter
+    def summary_variables(self, value):
+        """
+        Set summary variables
+
+        Parameters
+        ----------
+        value : list of strings
+            Names of the summary variables. Must all be in self.variables.
+        """
+        for var in value:
+            if var not in self.variables:
+                raise KeyError(
+                    f"No cycling variable defined for summary variable '{var}'"
+                )
+        self._summary_variables = value
+
+    def set_summary_variables(self):
+        self._summary_variables = []
+
     def set_external_circuit_submodel(self):
         """
         Define how the external circuit defines the boundary conditions for the model,

diff --git a/pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py b/pybamm/models/full_battery_models/lithium_ion/base_lithium_ion_model.py
@@ -118,6 +118,44 @@ def set_degradation_variables(self):
             }
         )
 
+    def set_summary_variables(self):
+        """
+        Sets the default summary variables.
+        """
+        summary_variables = [
+            "Positive electrode capacity [A.h]",
+            # LAM, LLI
+            "Loss of active material in positive electrode [%]",
+            "Loss of lithium inventory [%]",
+            "Loss of lithium inventory, including electrolyte [%]",
+            # Total lithium
+            "Total lithium [mol]",
+            "Total lithium in electrolyte [mol]",
+            "Total lithium in positive electrode [mol]",
+            "Total lithium in particles [mol]",
+            # Lithium lost
+            "Total lithium lost [mol]",
+            "Total lithium lost from particles [mol]",
+            "Total lithium lost from electrolyte [mol]",
+            "Loss of lithium to SEI [mol]",
+            "Loss of lithium to lithium plating [mol]",
+            "Loss of capacity to SEI [A.h]",
+            "Loss of capacity to lithium plating [A.h]",
+            "Total lithium lost to side reactions [mol]",
+            "Total capacity lost to side reactions [A.h]",
+            # Resistance
+            "Local ECM resistance [Ohm]",
+        ]
+
+        if not self.half_cell:
+            summary_variables += [
+                "Negative electrode capacity [A.h]",
+                "Loss of active material in negative electrode [%]",
+                "Total lithium in negative electrode [mol]",
+            ]
+
+        self.summary_variables = summary_variables
+
     def set_sei_submodel(self):
         if self.half_cell:
             reaction_loc = "interface"

diff --git a/pybamm/solvers/solution.py b/pybamm/solvers/solution.py
@@ -796,45 +796,25 @@ def make_cycle_solution(step_solutions, esoh_sim=None, save_this_cycle=True):
 
 
 def get_cycle_summary_variables(cycle_solution, esoh_sim):
-    Q = cycle_solution["Discharge capacity [A.h]"].data
-    min_Q = np.min(Q)
-    max_Q = np.max(Q)
-
-    cycle_summary_variables = pybamm.FuzzyDict(
-        {
-            "Minimum measured discharge capacity [A.h]": min_Q,
-            "Maximum measured discharge capacity [A.h]": max_Q,
-            "Measured capacity [A.h]": max_Q - min_Q,
-        }
-    )
+    model = cycle_solution.all_models[0]
+    cycle_summary_variables = pybamm.FuzzyDict({})
+
+    # Measured capacity variables
+    if "Discharge capacity [A.h]" in model.variables:
+        Q = cycle_solution["Discharge capacity [A.h]"].data
+        min_Q = np.min(Q)
+        max_Q = np.max(Q)
+
+        cycle_summary_variables.update(
+            {
+                "Minimum measured discharge capacity [A.h]": min_Q,
+                "Maximum measured discharge capacity [A.h]": max_Q,
+                "Measured capacity [A.h]": max_Q - min_Q,
+            }
+        )
 
-    degradation_variables = [
-        "Negative electrode capacity [A.h]",
-        "Positive electrode capacity [A.h]",
-        # LAM, LLI
-        "Loss of active material in negative electrode [%]",
-        "Loss of active material in positive electrode [%]",
-        "Loss of lithium inventory [%]",
-        "Loss of lithium inventory, including electrolyte [%]",
-        # Total lithium
-        "Total lithium [mol]",
-        "Total lithium in electrolyte [mol]",
-        "Total lithium in positive electrode [mol]",
-        "Total lithium in negative electrode [mol]",
-        "Total lithium in particles [mol]",
-        # Lithium lost
-        "Total lithium lost [mol]",
-        "Total lithium lost from particles [mol]",
-        "Total lithium lost from electrolyte [mol]",
-        "Loss of lithium to SEI [mol]",
-        "Loss of lithium to lithium plating [mol]",
-        "Loss of capacity to SEI [A.h]",
-        "Loss of capacity to lithium plating [A.h]",
-        "Total lithium lost to side reactions [mol]",
-        "Total capacity lost to side reactions [A.h]",
-        # Resistance
-        "Local ECM resistance [Ohm]",
-    ]
+    # Degradation variables
+    degradation_variables = model.summary_variables
     first_state = cycle_solution.first_state
     last_state = cycle_solution.last_state
     for var in degradation_variables:
@@ -846,7 +826,12 @@ def get_cycle_summary_variables(cycle_solution, esoh_sim):
             data_last[0] - data_first[0]
         )
 
-    if esoh_sim is not None:
+    # eSOH variables (full-cell lithium-ion model only, for now)
+    if (
+        esoh_sim is not None
+        and isinstance(model, pybamm.lithium_ion.BaseModel)
+        and model.half_cell is False
+    ):
         V_min = esoh_sim.parameter_values["Lower voltage cut-off [V]"]
         V_max = esoh_sim.parameter_values["Upper voltage cut-off [V]"]
         C_n = last_state["Negative electrode capacity [A.h]"].data[0]
@@ -887,19 +872,16 @@ def get_cycle_summary_variables(cycle_solution, esoh_sim):
             esoh_sim.built_model.set_initial_conditions_from(
                 {"x_100": x_100_init, "C": C_init}
             )
+        inputs = {
+            "V_min": V_min,
+            "V_max": V_max,
+            "C_n": C_n,
+            "C_p": C_p,
+            "n_Li": n_Li,
+        }
 
         try:
-            esoh_sol = esoh_sim.solve(
-                [0],
-                inputs={
-                    "V_min": V_min,
-                    "V_max": V_max,
-                    "C_n": C_n,
-                    "C_p": C_p,
-                    "n_Li": n_Li,
-                },
-                solver=solver,
-            )
+            esoh_sol = esoh_sim.solve([0], inputs=inputs, solver=solver)
         except pybamm.SolverError:  # pragma: no cover
             raise pybamm.SolverError(
                 "Could not solve for summary variables, run "

diff --git a/tests/unit/test_experiments/test_simulation_with_experiment.py b/tests/unit/test_experiments/test_simulation_with_experiment.py
@@ -350,6 +350,28 @@ def test_inputs(self):
         sim.solve(inputs={"Dsn": 2})
         np.testing.assert_array_equal(sim.solution.all_inputs[0]["Dsn"], 2)
 
+    def test_run_experiment_half_cell(self):
+        experiment = pybamm.Experiment(
+            [("Discharge at C/20 until 3.5V", "Charge at 1C until 3.8 V")]
+        )
+        model = pybamm.lithium_ion.DFN({"working electrode": "positive"})
+        sim = pybamm.Simulation(
+            model,
+            experiment=experiment,
+            parameter_values=pybamm.ParameterValues(
+                chemistry=pybamm.parameter_sets.Xu2019
+            ),
+        )
+        sim.solve()
+
+    def test_run_experiment_lead_acid(self):
+        experiment = pybamm.Experiment(
+            [("Discharge at C/20 until 1.9V", "Charge at 1C until 2.1 V")]
+        )
+        model = pybamm.lead_acid.Full()
+        sim = pybamm.Simulation(model, experiment=experiment)
+        sim.solve()
+
 
 if __name__ == "__main__":
     print("Add -v for more debug output")

diff --git a/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py b/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py
@@ -76,6 +76,14 @@ def test_process_parameters_and_discretise(self):
         disc_flux_2 = disc.process_symbol(param_flux_2)
         self.assertEqual(flux_1.id, disc_flux_2.id)
 
+    def test_summary_variables(self):
+        model = pybamm.BaseBatteryModel()
+        model.variables["var"] = pybamm.Scalar(1)
+        model.summary_variables = ["var"]
+        self.assertEqual(model.summary_variables, ["var"])
+        with self.assertRaisesRegex(KeyError, "No cycling variable defined"):
+            model.summary_variables = ["bad var"]
+
     def test_default_geometry(self):
         var = pybamm.standard_spatial_vars