From a2481fa48acc3afcafa0533db0ddbf59c8cec075 Mon Sep 17 00:00:00 2001
From: Robert Timms <43040151+rtimms@users.noreply.github.com>
Date: Mon, 15 Jan 2024 16:14:25 +0000
Subject: [PATCH] #3690 fix issue with skipped steps (#3708)

* #3690 fix issue with skipped steps

* #3690 changelog

* #3690 add test
---
 CHANGELOG.md                                  |  3 +++
 pybamm/simulation.py                          | 15 ++++++++++++++-
 .../test_simulation_with_experiment.py        | 19 +++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 965a2aa7b4..00ad65f9a0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # [Unreleased](https://github.com/pybamm-team/PyBaMM/)
 
+## Bug Fixes
+
+- Fixed a bug where if the first step(s) in a cycle are skipped then the cycle solution started from the model's initial conditions instead of from the last state of the previous cycle ([#3708](https://github.com/pybamm-team/PyBaMM/pull/3708))
 # [v24.1rc0](https://github.com/pybamm-team/PyBaMM/tree/v24.1rc0) - 2024-01-31
 
 ## Features
diff --git a/pybamm/simulation.py b/pybamm/simulation.py
index c95ab3039c..8a6150cc4e 100644
--- a/pybamm/simulation.py
+++ b/pybamm/simulation.py
@@ -839,7 +839,20 @@ def solve(
 
                     steps.append(step_solution)
 
-                    cycle_solution = cycle_solution + step_solution
+                    # If there haven't been any successful steps yet in this cycle, then
+                    # carry the solution over from the previous cycle (but
+                    # `step_solution` should still be an EmptySolution so that in the
+                    # list of returned step solutions we can see which steps were
+                    # skipped)
+                    if (
+                        cycle_solution is None
+                        and isinstance(step_solution, pybamm.EmptySolution)
+                        and not isinstance(current_solution, pybamm.EmptySolution)
+                    ):
+                        cycle_solution = current_solution.last_state
+                    else:
+                        cycle_solution = cycle_solution + step_solution
+
                     current_solution = cycle_solution
 
                     callbacks.on_step_end(logs)
diff --git a/tests/unit/test_experiments/test_simulation_with_experiment.py b/tests/unit/test_experiments/test_simulation_with_experiment.py
index cc04177ba2..36475081c3 100644
--- a/tests/unit/test_experiments/test_simulation_with_experiment.py
+++ b/tests/unit/test_experiments/test_simulation_with_experiment.py
@@ -519,6 +519,25 @@ def test_run_experiment_skip_steps(self):
                 decimal=5,
             )
 
+    def test_skipped_step_continuous(self):
+        model = pybamm.lithium_ion.SPM({"SEI": "solvent-diffusion limited"})
+        experiment = pybamm.Experiment(
+            [
+                ("Rest for 24 hours (1 hour period)",),
+                (
+                    "Charge at C/3 until 4.1 V",
+                    "Hold at 4.1V until C/20",
+                    "Discharge at C/3 until 2.5 V",
+                ),
+            ]
+        )
+        sim = pybamm.Simulation(model, experiment=experiment)
+        sim.solve(initial_soc=1)
+        np.testing.assert_array_almost_equal(
+            sim.solution.cycles[0].last_state.y.full(),
+            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")