Changed max_steps to default to -1

Author: EricWiener

pytorch_lightning/loops/epoch/training_epoch_loop.py

@@ -38,7 +38,7 @@ def __init__(self, min_steps: int, max_steps: int):
         super().__init__()
         self.min_steps: int = min_steps
 
-        if max_steps and max_steps < -1:
+        if max_steps < -1:
             raise MisconfigurationException(f"`max_steps` must be a positive integer or -1. You passed in {max_steps}.")
         self.max_steps: int = max_steps
 
@@ -75,7 +75,7 @@ def done(self) -> bool:
         The criteria are that the number of steps reached the max steps, the last batch is reached or the trainer
         signals to stop (e.g. by early stopping).
         """
-        max_steps_reached = self.max_steps is not None and self.global_step >= self.max_steps
+        max_steps_reached = self.max_steps != -1 and self.global_step >= self.max_steps
         return max_steps_reached or self.trainer.should_stop or self._num_training_batches_reached(self.is_last_batch)
 
     def connect(

pytorch_lightning/loops/fit_loop.py

@@ -33,14 +33,8 @@ class FitLoop(Loop):
         max_epochs: The maximum number of epochs
     """
 
-    def __init__(self, min_epochs: Optional[int] = None, max_epochs: Optional[int] = None):
+    def __init__(self, min_epochs: Optional[int] = None, max_epochs: int = None):
         super().__init__()
-        # Allow max_epochs or max_steps to be zero, since this will be handled by fit_loop.done
-        if max_epochs and max_epochs < -1:
-            raise MisconfigurationException(
-                f"`max_epochs` must be a positive integer or -1. You passed in {max_epochs}."
-            )
-
         self.max_epochs = max_epochs
         self.min_epochs = min_epochs
         self.epoch_loop: Optional[TrainingEpochLoop] = None
@@ -135,19 +129,6 @@ def _results(self) -> ResultCollection:
             return self.epoch_loop.val_loop._results
         raise RuntimeError("`FitLoop._results` property isn't defined. Accessed outside of scope")
 
-    @staticmethod
-    def _is_max_limit_enabled(max_value: Optional[int]) -> bool:
-        """Checks whether the max_value is enabled. This can be used for checking whether max_epochs or max_steps
-        is enabled.
-
-        Args:
-            max_value: the value to check
-
-        Returns:
-            whether the limit for this value should be enabled
-        """
-        return max_value not in (None, -1)
-
     @property
     def done(self) -> bool:
         """Evaluates when to leave the loop.
@@ -156,8 +137,8 @@ def done(self) -> bool:
         is reached.
         """
         # TODO(@awaelchli): Move track steps inside training loop and move part of these condition inside training loop
-        stop_steps = FitLoop._is_max_limit_enabled(self.max_steps) and self.global_step >= self.max_steps
-        stop_epochs = FitLoop._is_max_limit_enabled(self.max_epochs) and self.current_epoch >= self.max_epochs
+        stop_steps = self.max_steps != -1 and self.global_step >= self.max_steps
+        stop_epochs = self.max_epochs != -1 and self.current_epoch >= self.max_epochs
 
         should_stop = False
         if self.trainer.should_stop:

pytorch_lightning/trainer/connectors/checkpoint_connector.py

@@ -191,10 +191,7 @@ def restore_loops(self) -> None:
         self.trainer.fit_loop.current_epoch = self._loaded_checkpoint["epoch"]
 
         # crash if max_epochs is lower then the current epoch from the checkpoint
-        if (
-            FitLoop._is_max_limit_enabled(self.trainer.max_epochs)
-            and self.trainer.current_epoch > self.trainer.max_epochs
-        ):
+        if self.trainer.max_epochs != -1 and self.trainer.current_epoch > self.trainer.max_epochs:
             raise MisconfigurationException(
                 f"You restored a checkpoint with current_epoch={self.trainer.current_epoch},"
                 f" but you have set Trainer(max_epochs={self.trainer.max_epochs})."

pytorch_lightning/trainer/trainer.py

@@ -122,7 +122,7 @@ def __init__(
         accumulate_grad_batches: Union[int, Dict[int, int]] = 1,
         max_epochs: Optional[int] = None,
         min_epochs: Optional[int] = None,
-        max_steps: Optional[int] = None,
+        max_steps: int = -1,
         min_steps: Optional[int] = None,
         max_time: Optional[Union[str, timedelta, Dict[str, int]]] = None,
         limit_train_batches: Union[int, float] = 1.0,
@@ -273,9 +273,9 @@ def __init__(
             min_epochs: Force training for at least these many epochs. Disabled by default (None).
                 If both min_epochs and min_steps are not specified, defaults to ``min_epochs = 1``.
 
-            max_steps: Stop training after this number of steps. Disabled by default (None). If ``max_steps = None``
+            max_steps: Stop training after this number of steps. Disabled by default (-1). If ``max_steps = -1``
                 and ``max_epochs = None``, will default to ``max_epochs = 1000``. To disable this default, set
-                ``max_steps`` to ``-1``.
+                ``max_epochs`` to ``-1``.
 
             min_steps: Force training for at least these number of steps. Disabled by default (None).
 
@@ -382,10 +382,19 @@ def __init__(
         self.slurm_connector = SLURMConnector(self)
         self.tuner = Tuner(self)
 
-        # max_epochs won't default to 1000 if max_steps/max_time are specified (including being set to -1).
+        if max_epochs is None:
+            # max_epochs won't default to 1000 if max_steps/max_time are non-default values.
+            max_epochs = 1000 if (max_steps == -1 and max_time is None) else -1
+
+        elif max_epochs < -1:
+            # Allow max_epochs to be zero, since this will be handled by fit_loop.done
+            raise MisconfigurationException(
+                f"`max_epochs` must be a positive integer or -1. You passed in {max_epochs}."
+            )
+
         fit_loop = FitLoop(
             min_epochs=(1 if (min_epochs is None and min_steps is None and max_time is None) else min_epochs),
-            max_epochs=(1000 if (max_epochs is None and max_steps is None and max_time is None) else max_epochs),
+            max_epochs=max_epochs,
         )
         training_epoch_loop = TrainingEpochLoop(min_steps, max_steps)
         training_batch_loop = TrainingBatchLoop()

tests/callbacks/test_timer.py

@@ -49,7 +49,7 @@ def on_fit_start(self):
     timer = [c for c in trainer.callbacks if isinstance(c, Timer)][0]
     assert timer._duration == 1
     assert trainer.max_epochs == -1
-    assert trainer.max_steps is None
+    assert trainer.max_steps == -1
 
 
 @pytest.mark.parametrize(

tests/trainer/flags/test_env_vars.py

@@ -21,7 +21,8 @@ def test_passing_no_env_variables():
     """Testing overwriting trainer arguments."""
     trainer = Trainer()
     assert trainer.logger is not None
-    assert trainer.max_steps is None
+    assert trainer.max_steps == -1
+    assert trainer.max_epochs == 1000
     trainer = Trainer(False, max_steps=42)
     assert trainer.logger is None
     assert trainer.max_steps == 42

tests/trainer/optimization/test_optimizers.py

@@ -373,7 +373,7 @@ def test_lr_scheduler_strict(step_mock, tmpdir, complete_epoch):
     optimizer = optim.Adam(model.parameters())
     scheduler = optim.lr_scheduler.ReduceLROnPlateau(optimizer)
     max_epochs = 1 if complete_epoch else None
-    max_steps = None if complete_epoch else 1
+    max_steps = -1 if complete_epoch else 1
     trainer = Trainer(default_root_dir=tmpdir, max_epochs=max_epochs, max_steps=max_steps)
 
     model.configure_optimizers = lambda: {

tests/trainer/test_trainer.py

@@ -504,7 +504,7 @@ def test_trainer_max_steps_and_epochs(tmpdir):
 @pytest.mark.parametrize(
     "max_epochs,max_steps,incorrect_variable,incorrect_value",
     [
-        (-100, None, "max_epochs", -100),
+        (-100, -1, "max_epochs", -100),
         (1, -2, "max_steps", -2),
     ],
 )
@@ -520,13 +520,13 @@ def test_trainer_max_steps_and_epochs_validation(max_epochs, max_steps, incorrec
 @pytest.mark.parametrize(
     "max_epochs,max_steps,is_done,correct_trainer_epochs",
     [
-        (None, None, False, 1000),
-        (-1, None, False, -1),
+        (None, -1, False, 1000),
+        (-1, -1, False, -1),
         (None, -1, False, None),
         (5, -1, False, 5),
         (-1, 10, False, -1),
         (None, 0, True, None),
-        (0, None, True, 0),
+        (0, -1, True, 0),
         (-1, 0, True, -1),
         (0, -1, True, 0),
     ],