Repository: pybamm-team/PyBaMM
Pull request #4839 (2025-02-12 17:01:05)

add option to skip experiment steps
# Description

Adds an option to skip experiment steps. This is useful in CCCV if you start with too high of an initial SOC to run the CC step.

Fixes # (issue)

## Type of change

Please add a line in the relevant section of [CHANGELOG.md](https://github.com/pybamm-team/PyBaMM/blob/develop/CHANGELOG.md) to document the change (include PR #)

# Important checks:

Please confirm the following before marking the PR as ready for review:
- No style issues: `nox -s pre-commit`
- All tests pass: `nox -s tests`
- The documentation builds: `nox -s doctests`
- Code is commented for hard-to-understand areas
- Tests added that prove fix is effective or that feature works


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pybamm-team/PyBaMM/pull/4839?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pybamm-team) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 98.70%. Comparing base [(`2803090`)](https://app.codecov.io/gh/pybamm-team/PyBaMM/commit/2803090f20e1c6b1ef52f9a4d2226b0491f0091f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pybamm-team) to head [(`292840b`)](https://app.codecov.io/gh/pybamm-team/PyBaMM/commit/292840b9b6a2d7b43096b3b36798eab13bd6394e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pybamm-team).

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff            @@
##           develop    #4839   +/-   ##
========================================
  Coverage    98.70%   98.70%           
========================================
  Files          303      303           
  Lines        23328    23333    +5     
========================================
+ Hits         23025    23030    +5     
  Misses         303      303           
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/pybamm-team/PyBaMM/pull/4839?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pybamm-team).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pybamm-team).

kratman: @aabills I made this a draft so that it was not accidentally merged while you address Valentin's comments
aabills: Thanks yeah this is going to take a bit longer because it substantially changes behavior in a few tests.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0f08fd8bc..1df6231fb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@
 
 ## Breaking changes
 
+- Added `skip_ok` option to `step` to allow for steps to be skipped if they are infeasible at initial conditions. ([#4839](https://github.com/pybamm-team/PyBaMM/pull/4839))
 - Deprecated `CrateTermination` and renamed it to `CRateTermination`. ([#4834](https://github.com/pybamm-team/PyBaMM/pull/4834))
 
 ## Bug fixes
diff --git a/src/pybamm/callbacks.py b/src/pybamm/callbacks.py
index 29286ab5a..cece27c84 100644
--- a/src/pybamm/callbacks.py
+++ b/src/pybamm/callbacks.py
@@ -195,7 +195,7 @@ class LoggingCallback(Callback):
                 )
 
     def on_cycle_end(self, logs):
-        cap_stop = logs["stopping conditions"]["capacity"]
+        cap_stop = logs["stopping conditions"].get("capacity")
         if cap_stop is not None:
             cap_now = logs["summary variables"]["Capacity [A.h]"]
             cap_start = logs["start capacity"]
diff --git a/src/pybamm/experiment/step/base_step.py b/src/pybamm/experiment/step/base_step.py
index b2f92d4df..e6dc1fd88 100644
--- a/src/pybamm/experiment/step/base_step.py
+++ b/src/pybamm/experiment/step/base_step.py
@@ -55,6 +55,9 @@ class BaseStep:
         A description of the step.
     direction : str, optional
         The direction of the step, e.g. "Charge" or "Discharge" or "Rest".
+    skip_ok : bool, optional
+        If True, the step will be skipped if it is infeasible at the initial conditions.
+        Default is True.
     """
 
     def __init__(
@@ -68,6 +71,7 @@ class BaseStep:
         start_time=None,
         description=None,
         direction: str | None = None,
+        skip_ok: bool = True,
     ):
         potential_directions = ["charge", "discharge", "rest", None]
         if direction not in potential_directions:
@@ -77,6 +81,7 @@ class BaseStep:
         self.input_duration = duration
         self.input_duration = duration
         self.input_value = value
+        self.skip_ok = skip_ok
         # Check if drive cycle
         is_drive_cycle = isinstance(value, np.ndarray)
         is_python_function = callable(value)
@@ -213,6 +218,7 @@ class BaseStep:
             start_time=self.start_time,
             description=self.description,
             direction=self.direction,
+            skip_ok=self.skip_ok,
         )
 
     def __str__(self):
diff --git a/src/pybamm/simulation.py b/src/pybamm/simulation.py
index 448b750d0..8ca5d0f79 100644
--- a/src/pybamm/simulation.py
+++ b/src/pybamm/simulation.py
@@ -779,7 +779,8 @@ class Simulation:
                             and "[experiment]" in error.message
                         ):
                             step_solution = pybamm.EmptySolution(
-                                "Event exceeded in initial conditions", t=start_time
+                                "Event exceeded in initial conditions",
+                                t=start_time,
                             )
                         else:
                             logs["error"] = error
@@ -877,25 +878,47 @@ class Simulation:
                 # At the final step of the inner loop we save the cycle
                 if len(steps) > 0:
                     # Check for EmptySolution
-                    if all(isinstance(step, pybamm.EmptySolution) for step in steps):
+                    if all(
+                        isinstance(step_solution, pybamm.EmptySolution)
+                        for step_solution in steps
+                    ):
                         if len(steps) == 1:
-                            raise pybamm.SolverError(
-                                f"Step '{step_str}' is infeasible "
-                                "due to exceeded bounds at initial conditions. "
-                                "If this step is part of a longer cycle, "
-                                "round brackets should be used to indicate this, "
-                                "e.g.:\n pybamm.Experiment([(\n"
-                                "\tDischarge at C/5 for 10 hours or until 3.3 V,\n"
-                                "\tCharge at 1 A until 4.1 V,\n"
-                                "\tHold at 4.1 V until 10 mA\n"
-                                "])"
-                            )
+                            if step.skip_ok:
+                                pybamm.logger.warning(
+                                    f"Step '{step_str}' is infeasible at initial conditions, but skip_ok is True. Skipping step."
+                                )
+                                continue
+                            else:
+                                raise pybamm.SolverError(
+                                    f"Step '{step_str}' is infeasible "
+                                    "due to exceeded bounds at initial conditions. "
+                                    "If this step is part of a longer cycle, "
+                                    "round brackets should be used to indicate this, "
+                                    "e.g.:\n pybamm.Experiment([(\n"
+                                    "\tDischarge at C/5 for 10 hours or until 3.3 V,\n"
+                                    "\tCharge at 1 A until 4.1 V,\n"
+                                    "\tHold at 4.1 V until 10 mA\n"
+                                    "])\n"
+                                    "Otherwise, set skip_ok=True when instantiating the step to skip this step."
+                                )
                         else:
                             this_cycle = self.experiment.cycles[cycle_num - 1]
-                            raise pybamm.SolverError(
-                                f"All steps in the cycle {this_cycle} are infeasible "
-                                "due to exceeded bounds at initial conditions."
+                            all_steps_skipped = all(
+                                this_step.skip_ok
+                                for this_step in this_cycle
+                                if isinstance(this_step, pybamm.step.BaseStep)
                             )
+                            if all_steps_skipped:
+                                raise pybamm.SolverError(
+                                    f"All steps in the cycle {this_cycle} are infeasible "
+                                    "due to exceeded bounds at initial conditions, though "
+                                    "skip_ok is True for all steps. Please recheck the experiment."
+                                )
+                            else:
+                                raise pybamm.SolverError(
+                                    f"All steps in the cycle {this_cycle} are infeasible "
+                                    "due to exceeded bounds at initial conditions."
+                                )
                     cycle_sol = pybamm.make_cycle_solution(
                         steps,
                         esoh_solver=esoh_solver,
@@ -924,6 +947,8 @@ class Simulation:
                     else:
                         capacity_stop = None
                     logs["stopping conditions"]["capacity"] = capacity_stop
+                else:
+                    capacity_stop = logs["stopping conditions"].get("capacity")
 
                 logs["elapsed time"] = timer.time()
 


Test patch:
diff --git a/tests/unit/test_experiments/test_simulation_with_experiment.py b/tests/unit/test_experiments/test_simulation_with_experiment.py
index 89292cec4..dfc24ac3e 100644
--- a/tests/unit/test_experiments/test_simulation_with_experiment.py
+++ b/tests/unit/test_experiments/test_simulation_with_experiment.py
@@ -145,6 +145,63 @@ class TestSimulationExperiment:
         assert len(sol3.cycles) == 2
         os.remove("test_experiment.sav")
 
+    def test_skip_ok(self):
+        model = pybamm.lithium_ion.SPMe()
+        cc_charge_skip_ok = pybamm.step.Current(-5, termination="4.2 V")
+        cc_charge_skip_not_ok = pybamm.step.Current(
+            -5, termination="4.2 V", skip_ok=False
+        )
+        steps = [
+            pybamm.step.Current(2, duration=100.0, skip_ok=False),
+            cc_charge_skip_ok,
+            pybamm.step.Voltage(4.2, termination="0.01 A", skip_ok=False),
+        ]
+        param = pybamm.ParameterValues("Chen2020")
+        experiment = pybamm.Experiment(steps)
+        sim = pybamm.Simulation(model, experiment=experiment, parameter_values=param)
+        sol = sim.solve()
+        # Make sure we know to skip it if we should and not if we shouldn't
+        assert sim.experiment.steps[1].skip_ok
+        assert not sim.experiment.steps[0].skip_ok
+
+        # Make sure we actually skipped it because it is infeasible
+        assert len(sol.cycles) == 2
+
+        # In this case, it is feasible, so we should not skip it
+        sol2 = sim.solve(initial_soc=0.5)
+        assert len(sol2.cycles) == 3
+
+        # make sure we raise an error if we shouldn't skip it and it is infeasible
+        steps[1] = cc_charge_skip_not_ok
+        experiment = pybamm.Experiment(steps)
+        sim = pybamm.Simulation(model, experiment=experiment, parameter_values=param)
+        with pytest.raises(pybamm.SolverError):
+            sim.solve()
+
+        # make sure we raise an error if all steps are infeasible
+        steps = [
+            (pybamm.step.Current(-5, termination="4.2 V", skip_ok=True),) * 5,
+        ]
+        experiment = pybamm.Experiment(steps)
+        sim = pybamm.Simulation(model, experiment=experiment, parameter_values=param)
+        with pytest.raises(pybamm.SolverError, match="skip_ok is True for all steps"):
+            sim.solve()
+
+    def test_all_empty_solution_errors(self):
+        model = pybamm.lithium_ion.SPM()
+        parameter_values = pybamm.ParameterValues("Chen2020")
+
+        # One step exceeded, suggests making a cycle
+        steps = [
+            (pybamm.step.Current(-5, termination="4.2 V", skip_ok=False),) * 5,
+        ]
+        experiment = pybamm.Experiment(steps)
+        sim = pybamm.Simulation(
+            model, experiment=experiment, parameter_values=parameter_values
+        )
+        with pytest.raises(pybamm.SolverError, match="All steps in the cycle"):
+            sim.solve()
+
     def test_run_experiment_multiple_times(self):
         s = pybamm.step.string
         experiment = pybamm.Experiment(
@@ -679,31 +736,6 @@ class TestSimulationExperiment:
             sim.solution.cycles[1].steps[-1].first_state.y.full(),
         )
 
-    def test_all_empty_solution_errors(self):
-        model = pybamm.lithium_ion.SPM()
-        parameter_values = pybamm.ParameterValues("Chen2020")
-
-        # One step exceeded, suggests making a cycle
-        experiment = pybamm.Experiment([("Charge at 1C until 4.2V")])
-        sim = pybamm.Simulation(
-            model, parameter_values=parameter_values, experiment=experiment
-        )
-        with pytest.raises(
-            pybamm.SolverError,
-            match="Step 'Charge at 1C until 4.2V' is infeasible due to exceeded bounds",
-        ):
-            sim.solve()
-
-        # Two steps exceeded, different error
-        experiment = pybamm.Experiment(
-            [("Charge at 1C until 4.2V", "Charge at 1C until 4.2V")]
-        )
-        sim = pybamm.Simulation(
-            model, parameter_values=parameter_values, experiment=experiment
-        )
-        with pytest.raises(pybamm.SolverError, match="All steps in the cycle"):
-            sim.solve()
-
     def test_solver_error(self):
         model = pybamm.lithium_ion.DFN()  # load model
         parameter_values = pybamm.ParameterValues("Chen2020")