- Merged flags "should_terminate" and "_skip_completed_after_termination".

- Merged flags "should_terminate_single_epoch" and "_skip_epoch_completed_after_termination".

Author: bonassifabio

Diff for: ignite/engine/engine.py

@@ -139,10 +139,12 @@ def __init__(self, process_function: Callable[["Engine", Any], Any]):
         self.logger = logging.getLogger(__name__ + "." + self.__class__.__name__)
         self._process_function = process_function
         self.last_event_name: Optional[Events] = None
-        self.should_terminate = False
-        self._skip_completed_after_termination = False
-        self.should_terminate_single_epoch = False
-        self._skip_epoch_completed_after_termination = False
+        # should_terminate flag: False - don't terminate, True - terminate,
+        # "skip_completed" - terminate and skip the event "COMPLETED"
+        self.should_terminate: bool | str = False
+        # should_terminate_single_epoch flag: False - don't terminate, True - terminate,
+        # "skip_epoch_completed" - terminate and skip the event "EPOCH_COMPLETED"
+        self.should_terminate_single_epoch: bool | str = False
         self.should_interrupt = False
         self.state = State()
         self._state_dict_user_keys: List[str] = []
@@ -626,8 +628,7 @@ def terminate():
             Added `skip_completed` flag
         """
         self.logger.info("Terminate signaled. Engine will stop after current iteration is finished.")
-        self.should_terminate = True
-        self._skip_completed_after_termination = skip_completed
+        self.should_terminate = "skip_completed" if skip_completed else True
 
     def terminate_epoch(self, skip_epoch_completed: bool = False) -> None:
         """Sends terminate signal to the engine, so that it terminates the current epoch. The run
@@ -651,8 +652,7 @@ def terminate_epoch(self, skip_epoch_completed: bool = False) -> None:
             "Terminate current epoch is signaled. "
             "Current epoch iteration will stop after current iteration is finished."
         )
-        self.should_terminate_single_epoch = True
-        self._skip_epoch_completed_after_termination = skip_epoch_completed
+        self.should_terminate_single_epoch = "skip_epoch_completed" if skip_epoch_completed else True
 
     def _handle_exception(self, e: BaseException) -> None:
         if Events.EXCEPTION_RAISED in self._event_handlers:
@@ -991,15 +991,14 @@ def _internal_run_as_gen(self) -> Generator[Any, None, State]:
                     # time is available for handlers but must be updated after fire
                     self.state.times[Events.EPOCH_COMPLETED.name] = epoch_time_taken
 
-                    if not self._skip_epoch_completed_after_termination:
+                    if self.should_terminate_single_epoch != "skip_epoch_completed":
                         handlers_start_time = time.time()
                         self._fire_event(Events.EPOCH_COMPLETED)
                         epoch_time_taken += time.time() - handlers_start_time
                         # update time wrt handlers
                         self.state.times[Events.EPOCH_COMPLETED.name] = epoch_time_taken
-                    else:
-                        self._skip_epoch_completed_after_termination = False
 
+                    self.should_terminate_single_epoch = False
                     yield from self._maybe_terminate_or_interrupt()
 
                     hours, mins, secs = _to_hours_mins_secs(epoch_time_taken)
@@ -1015,7 +1014,7 @@ def _internal_run_as_gen(self) -> Generator[Any, None, State]:
             self.state.times[Events.COMPLETED.name] = time_taken
 
             # do not fire Events.COMPLETED if we terminated the run with flag `skip_completed=True`
-            if not (self.should_terminate and self._skip_completed_after_termination):
+            if self.should_terminate != "skip_completed":
                 handlers_start_time = time.time()
                 self._fire_event(Events.COMPLETED)
                 time_taken += time.time() - handlers_start_time
@@ -1134,7 +1133,6 @@ def _run_once_on_dataset_as_gen(self) -> Generator[State, None, float]:
 
         except _EngineTerminateSingleEpochException:
             self._fire_event(Events.TERMINATE_SINGLE_EPOCH, iter_counter=iter_counter)
-            self.should_terminate_single_epoch = False
             self._setup_dataloader_iter()
 
         except _EngineTerminateException as e:
@@ -1180,15 +1178,14 @@ def _internal_run_legacy(self) -> State:
                     # time is available for handlers but must be updated after fire
                     self.state.times[Events.EPOCH_COMPLETED.name] = epoch_time_taken
 
-                    if not self._skip_epoch_completed_after_termination:
+                    if self.should_terminate_single_epoch != "skip_epoch_completed":
                         handlers_start_time = time.time()
                         self._fire_event(Events.EPOCH_COMPLETED)
                         epoch_time_taken += time.time() - handlers_start_time
                         # update time wrt handlers
                         self.state.times[Events.EPOCH_COMPLETED.name] = epoch_time_taken
-                    else:
-                        self._skip_epoch_completed_after_termination = False
 
+                    self.should_terminate_single_epoch = False
                     self._maybe_terminate_legacy()
 
                     hours, mins, secs = _to_hours_mins_secs(epoch_time_taken)
@@ -1204,7 +1201,7 @@ def _internal_run_legacy(self) -> State:
             self.state.times[Events.COMPLETED.name] = time_taken
 
             # do not fire Events.COMPLETED if we terminated the run with flag `skip_completed=True`
-            if not (self.should_terminate and self._skip_completed_after_termination):
+            if self.should_terminate != "skip_completed":
                 handlers_start_time = time.time()
                 self._fire_event(Events.COMPLETED)
                 time_taken += time.time() - handlers_start_time
@@ -1309,7 +1306,6 @@ def _run_once_on_dataset_legacy(self) -> float:
 
         except _EngineTerminateSingleEpochException:
             self._fire_event(Events.TERMINATE_SINGLE_EPOCH, iter_counter=iter_counter)
-            self.should_terminate_single_epoch = False
             self._setup_dataloader_iter()
 
         except _EngineTerminateException as e:

Diff for: tests/ignite/engine/test_engine.py

@@ -44,10 +44,13 @@ def set_interrupt_resume_enabled(self, interrupt_resume_enabled):
     def test_terminate(self, skip_completed):
         engine = Engine(lambda e, b: 1)
         assert not engine.should_terminate
-        assert not engine._skip_completed_after_termination
+
         engine.terminate(skip_completed)
-        assert engine.should_terminate
-        assert engine._skip_completed_after_termination == skip_completed
+
+        if skip_completed:
+            assert engine.should_terminate == "skip_completed"
+        else:
+            assert engine.should_terminate == True  # noqa: E712
 
     def test_invalid_process_raises_with_invalid_signature(self):
         with pytest.raises(ValueError, match=r"Engine must be given a processing function in order to run"):
@@ -338,27 +341,38 @@ def test_terminate_epoch_events_sequence(self, terminate_epoch_event, i, skip_ep
 
         @engine.on(terminate_epoch_event)
         def call_terminate_epoch():
-            assert not engine._skip_epoch_completed_after_termination
+            assert not engine.should_terminate_single_epoch
             nonlocal call_count
             if call_count < 1:
                 engine.terminate_epoch(skip_epoch_completed)
-                assert engine._skip_epoch_completed_after_termination == skip_epoch_completed
+                if skip_epoch_completed:
+                    assert engine.should_terminate_single_epoch == "skip_epoch_completed"
+                else:
+                    assert engine.should_terminate_single_epoch == True  # noqa: E712
 
             call_count += 1
 
+        @engine.on(Events.EPOCH_STARTED)
+        def check_skip_reset():
+            assert engine.should_terminate_single_epoch == False  # noqa: E712
+
         @engine.on(Events.TERMINATE_SINGLE_EPOCH)
         def check_previous_events(iter_counter):
             e = i // len(data) + 1
             assert engine.called_events[0] == (0, 0, Events.STARTED)
             assert engine.called_events[-2] == (e, i, terminate_epoch_event)
             assert engine.called_events[-1] == (e, i, Events.TERMINATE_SINGLE_EPOCH)
-            assert engine._skip_epoch_completed_after_termination == skip_epoch_completed
+            if skip_epoch_completed:
+                assert engine.should_terminate_single_epoch == "skip_epoch_completed"
+            else:
+                assert engine.should_terminate_single_epoch == True  # noqa: E712
 
         @engine.on(Events.EPOCH_COMPLETED)
         def check_previous_events2():
             e = i // len(data) + 1
             if e == engine.state.epoch and i == engine.state.iteration:
                 assert not skip_epoch_completed
+                assert isinstance(engine.should_terminate_single_epoch, bool)
                 assert engine.called_events[-3] == (e, i, terminate_epoch_event)
                 assert engine.called_events[-2] == (e, i, Events.TERMINATE_SINGLE_EPOCH)
                 assert engine.called_events[-1] == (e, i, Events.EPOCH_COMPLETED)