Fix bug with identical steps with different end times

pybamm/experiment/experiment.py

@@ -78,7 +78,7 @@ def __init__(
         self.operating_conditions_cycles = operating_conditions_cycles
         self.cycle_lengths = [len(cycle) for cycle in operating_conditions_cycles]
 
-        operating_conditions_steps_unprocessed = self._set_next_start_time(
+        self.operating_conditions_steps_unprocessed = self._set_next_start_time(
             [cond for cycle in operating_conditions_cycles for cond in cycle]
         )
 
@@ -89,7 +89,7 @@ def __init__(
         self.temperature = _convert_temperature_to_kelvin(temperature)
 
         processed_steps = {}
-        for step in operating_conditions_steps_unprocessed:
+        for step in self.operating_conditions_steps_unprocessed:
             if repr(step) in processed_steps:
                 continue
             elif isinstance(step, str):
@@ -106,7 +106,7 @@ def __init__(
 
         self.operating_conditions_steps = [
             processed_steps[repr(step)]
-            for step in operating_conditions_steps_unprocessed
+            for step in self.operating_conditions_steps_unprocessed
         ]
 
         # Save the processed unique steps and the processed operating conditions

pybamm/simulation.py

@@ -769,14 +769,19 @@ def solve(
                     # human-intuitive
                     op_conds = self.experiment.operating_conditions_steps[idx]
 
+                    # Hacky patch to allow correct processing of end_time and next_starting time
+                    # For efficiency purposes, op_conds treats identical steps as the same object
+                    # regardless of the initial time. Should be refactored as part of #3176
+                    op_conds_unproc = self.experiment.operating_conditions_steps_unprocessed[idx]
+
                     start_time = current_solution.t[-1]
 
                     # If step has an end time, dt must take that into account
-                    if op_conds.end_time:
+                    if getattr(op_conds_unproc, "end_time", None):
                         dt = min(
                             op_conds.duration,
                             (
-                                op_conds.end_time
+                                op_conds_unproc.end_time
                                 - (
                                     initial_start_time
                                     + timedelta(seconds=float(start_time))
@@ -829,9 +834,9 @@ def solve(
                     step_termination = step_solution.termination
 
                     # Add a padding rest step if necessary
-                    if op_conds.next_start_time is not None:
+                    if getattr(op_conds_unproc, "next_start_time", None) is not None:
                         rest_time = (
-                            op_conds.next_start_time
+                            op_conds_unproc.next_start_time
                             - (
                                 initial_start_time
                                 + timedelta(seconds=float(step_solution.t[-1]))

pybamm/step/_steps_util.py

@@ -70,6 +70,7 @@ def __init__(
         description=None,
     ):
         self.type = typ
+        self.raw_termination = termination
 
         # Record all the args for repr and hash
         self.repr_args = f"{typ}, {value}"
@@ -171,6 +172,19 @@ def basic_repr(self):
         """
         return f"_Step({self.hash_args})"
 
+    def copy(self):
+        return _Step(
+            typ=self.type,
+            value=self.value,
+            duration=self.duration,
+            termination=self.raw_termination,
+            period=self.period,
+            temperature=self.temperature,
+            tags=self.tags,
+            start_time=self.start_time,
+            description=self.description,
+        )
+
     def to_dict(self):
         """
         Convert the step to a dictionary.

tests/unit/test_experiments/test_experiment.py

@@ -183,41 +183,49 @@ def test_no_initial_start_time(self):
             )
 
     def test_set_next_start_time(self):
-        # Defined dummy experiment to access _set_next_start_time
-        experiment = pybamm.Experiment(["Rest for 1 hour"])
         raw_op = [
             pybamm.step._Step(
                 "current", 1, duration=3600, start_time=datetime(2023, 1, 1, 8, 0)
             ),
+            pybamm.step._Step("voltage", 2.5, duration=3600, start_time=None),
             pybamm.step._Step(
                 "current", 1, duration=3600, start_time=datetime(2023, 1, 1, 12, 0)
             ),
             pybamm.step._Step("current", 1, duration=3600, start_time=None),
+            pybamm.step._Step("voltage", 2.5, duration=3600, start_time=None),
             pybamm.step._Step(
                 "current", 1, duration=3600, start_time=datetime(2023, 1, 1, 15, 0)
             ),
         ]
+        experiment = pybamm.Experiment(raw_op)
         processed_op = experiment._set_next_start_time(raw_op)
 
         expected_next = [
+            None,
             datetime(2023, 1, 1, 12, 0),
             None,
+            None,
             datetime(2023, 1, 1, 15, 0),
             None,
         ]
 
         expected_end = [
             datetime(2023, 1, 1, 12, 0),
+            datetime(2023, 1, 1, 12, 0),
+            datetime(2023, 1, 1, 15, 0),
             datetime(2023, 1, 1, 15, 0),
             datetime(2023, 1, 1, 15, 0),
             None,
         ]
 
+        # Test method directly
         for next, end, op in zip(expected_next, expected_end, processed_op):
             # useful form for debugging
             self.assertEqual(op.next_start_time, next)
             self.assertEqual(op.end_time, end)
 
+        # TODO: once #3176 is completed, the test should pass for
+        # operating_conditions_steps (or equivalent) as well
 
 if __name__ == "__main__":
     print("Add -v for more debug output")