Repository: pytorch/ignite

Issue:
Engine restarts training from scratch unexpected
## 🐛 Bug description

Engine `_is_done` function only takes into account `max_epochs` parameter read from the state of the object. This leads to the followling unexpected behavior:

1. Instantiate a simple engine, train for 2 epochs (set max_epochs=2). Save checkpoints.
2. Train for 2 more epochs (set max_epochs=4) using last checkpoint.
3. This leads to the training restarting from scratch instead of continuing from the last checkpoint.

Why?

Engine.run functions has a `max_epochs` parameter that should have the precedence over the `max_epochs` stored in the checkpoint state in the [_is_done](https://github.com/pytorch/ignite/blob/master/ignite/engine/engine.py#L605). 
Currently, at runtime, to check if the training is done, we consider `max_epochs` from the state of the checkpoint, not the `max_epochs` parameter from the user. Therefore if the user specifies a different value (in the case he wants to train for more epochs), his input is not taken into account.

Here is a possible workaround to explain how to solve the problem:
```
    @staticmethod
    def _is_done(state, max_epochs=None):
        return state.iteration == state.epoch_length * (state.max_epochs if max_epochs is None else max_epochs)
```


## Environment

 - PyTorch Version (e.g., 1.4): 1.4.0
 - Ignite Version (e.g., 0.3.0): 0.4.0
 - OS (e.g., Linux): macOS
 - How you installed Ignite (`conda`, `pip`, source): pip
 - Python version: 3.6.10
 - Any other relevant information:



Discussion:
sdesrozis: CI ko 😔 

Patch:
diff --git a/ignite/engine/engine.py b/ignite/engine/engine.py
index 825cccd8..40f19cf8 100644
--- a/ignite/engine/engine.py
+++ b/ignite/engine/engine.py
@@ -552,11 +552,12 @@ class Engine:
             data (Iterable): Collection of batches allowing repeated iteration (e.g., list or `DataLoader`).
             max_epochs (int, optional): Max epochs to run for (default: None).
                 If a new state should be created (first run or run again from ended engine), it's default value is 1.
-                This argument should be `None` if run is resuming from a state.
+                If run is resuming from a state, provided `max_epochs` will be taken into account and should be larger
+                than `engine.state.max_epochs`.
             epoch_length (int, optional): Number of iterations to count as one epoch. By default, it can be set as
                 `len(data)`. If `data` is an iterator and `epoch_length` is not set, then it will be automatically
                 determined as the iteration on which data iterator raises `StopIteration`.
-                This argument should be `None` if run is resuming from a state.
+                This argument should not change if run is resuming from a state.
             seed (int, optional): Deprecated argument. Please, use `torch.manual_seed` or
                 :meth:`~ignite.utils.manual_seed`.
 
@@ -582,20 +583,8 @@ class Engine:
                 "Please, use torch.manual_seed or ignite.utils.manual_seed"
             )
 
-        if self.state is None or self._is_done(self.state):
-            # Create new state
-            if max_epochs is None:
-                max_epochs = 1
-            if epoch_length is None:
-                if hasattr(data, "__len__"):
-                    epoch_length = len(data)
-                    if epoch_length < 1:
-                        raise ValueError("Input data has zero size. Please provide non-empty data")
-
-            self.state = State(iteration=0, epoch=0, max_epochs=max_epochs, epoch_length=epoch_length)
-            self.logger.info("Engine run starting with max_epochs={}.".format(max_epochs))
-        else:
-            # Keep actual state and override it if input args provided
+        if self.state is not None:
+            # Check and apply overridden parameters
             if max_epochs is not None:
                 if max_epochs < self.state.epoch:
                     raise ValueError(
@@ -610,6 +599,20 @@ class Engine:
                             epoch_length, self.state.epoch_length
                         )
                     )
+
+        if self.state is None or self._is_done(self.state):
+            # Create new state
+            if max_epochs is None:
+                max_epochs = 1
+            if epoch_length is None:
+                if hasattr(data, "__len__"):
+                    epoch_length = len(data)
+                    if epoch_length < 1:
+                        raise ValueError("Input data has zero size. Please provide non-empty data")
+
+            self.state = State(iteration=0, epoch=0, max_epochs=max_epochs, epoch_length=epoch_length)
+            self.logger.info("Engine run starting with max_epochs={}.".format(max_epochs))
+        else:
             self.logger.info(
                 "Engine run resuming from iteration {}, epoch {} until {} epochs".format(
                     self.state.iteration, self.state.epoch, self.state.max_epochs


Test patch:
diff --git a/tests/ignite/engine/test_engine_state_dict.py b/tests/ignite/engine/test_engine_state_dict.py
index 9250001b..0f06dcf9 100644
--- a/tests/ignite/engine/test_engine_state_dict.py
+++ b/tests/ignite/engine/test_engine_state_dict.py
@@ -150,6 +150,26 @@ def test_load_state_dict_with_params_overriding_integration():
         engine.run(data, epoch_length=90)
 
 
+def test_continue_training():
+    # Tests issue : https://github.com/pytorch/ignite/issues/993
+    max_epochs = 2
+    data = range(10)
+    engine = Engine(lambda e, b: 1)
+    state = engine.run(data, max_epochs=max_epochs)
+    assert state.max_epochs == max_epochs
+    assert state.iteration == len(data) * max_epochs
+    assert state.epoch == max_epochs
+
+    @engine.on(Events.STARTED)
+    def assert_continue_training():
+        assert engine.state.epoch == max_epochs
+
+    state = engine.run(data, max_epochs=max_epochs * 2)
+    assert state.max_epochs == max_epochs * 2
+    assert state.iteration == len(data) * max_epochs * 2
+    assert state.epoch == max_epochs * 2
+
+
 def test_state_dict_with_user_keys_integration(dirname):
     engine = Engine(lambda e, b: 1)
     engine.state_dict_user_keys.append("alpha")