Merge branch 'master' into feature/trainer-validate-2

Author: carmocca

CHANGELOG.md

@@ -104,34 +104,42 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Expose DeepSpeed loss parameters to allow users to fix loss instability ([#6115](https://github.com/PyTorchLightning/pytorch-lightning/pull/6115))
 
 
-- Fixed `AttributeError` when `logger=None` on TPU ([#6221](https://github.com/PyTorchLightning/pytorch-lightning/pull/6221))
+- Fixed duplicate logs appearing in console when using the python logging module ([#5509](https://github.com/PyTorchLightning/pytorch-lightning/pull/5509), [#6275](https://github.com/PyTorchLightning/pytorch-lightning/pull/6275))
 
 
-- Fixed `ModelPruning(make_pruning_permanent=True)` pruning buffers getting removed when saved during training ([#6073](https://github.com/PyTorchLightning/pytorch-lightning/pull/6073))
+- Fixed DP reduction with collection ([#6324](https://github.com/PyTorchLightning/pytorch-lightning/pull/6324))
 
 
-- Fixed `trainer.test` from `best_path` hangs after calling `trainer.fit`  ([#6272](https://github.com/PyTorchLightning/pytorch-lightning/pull/6272))
+- Fixed `.teardown(stage='fit')` getting called during `trainer.test` ([#6386](https://github.com/PyTorchLightning/pytorch-lightning/pull/6386))
 
 
-- Fixed duplicate logs appearing in console when using the python logging module ([#5509](https://github.com/PyTorchLightning/pytorch-lightning/pull/5509), [#6275](https://github.com/PyTorchLightning/pytorch-lightning/pull/6275))
+- Fixed `.on_fit_{start,end}()` getting called during `trainer.test` ([#6386](https://github.com/PyTorchLightning/pytorch-lightning/pull/6386))
 
 
-- Fixed `SingleTPU` calling `all_gather` ([#6296](https://github.com/PyTorchLightning/pytorch-lightning/pull/6296))
+- Fixed an issue where the tuner would not tune the learning rate if also tuning the batch size ([#4688](https://github.com/PyTorchLightning/pytorch-lightning/pull/4688))
 
 
-- Fixed DP reduction with collection ([#6324](https://github.com/PyTorchLightning/pytorch-lightning/pull/6324))
+- Fixed logger creating directory structure too early in DDP ([#6380](https://github.com/PyTorchLightning/pytorch-lightning/pull/6380))
 
 
-- Fixed `.teardown(stage='fit')` getting called during `trainer.test` ([#6386](https://github.com/PyTorchLightning/pytorch-lightning/pull/6386))
-
-
-- Fixed `.on_fit_{start,end}()` getting called during `trainer.test` ([#6386](https://github.com/PyTorchLightning/pytorch-lightning/pull/6386))
+## [1.2.3] - 2021-03-09
 
+### Fixed
 
+- Fixed `ModelPruning(make_pruning_permanent=True)` pruning buffers getting removed when saved during training ([#6073](https://github.com/PyTorchLightning/pytorch-lightning/pull/6073))
+- Fixed when `_stable_1d_sort` to work when `n >= N` ([#6177](https://github.com/PyTorchLightning/pytorch-lightning/pull/6177))
+- Fixed `AttributeError` when `logger=None` on TPU ([#6221](https://github.com/PyTorchLightning/pytorch-lightning/pull/6221))
 - Fixed PyTorch Profiler with `emit_nvtx` ([#6260](https://github.com/PyTorchLightning/pytorch-lightning/pull/6260))
+- Fixed `trainer.test` from `best_path` hangs after calling `trainer.fit`  ([#6272](https://github.com/PyTorchLightning/pytorch-lightning/pull/6272))
+- Fixed `SingleTPU` calling `all_gather` ([#6296](https://github.com/PyTorchLightning/pytorch-lightning/pull/6296))
+- Ensure we check deepspeed/sharded in multinode DDP ([#6297](https://github.com/PyTorchLightning/pytorch-lightning/pull/6297)
+- Check `LightningOptimizer` doesn't delete optimizer hooks ([#6305](https://github.com/PyTorchLightning/pytorch-lightning/pull/6305)
+- Resolve memory leak for evaluation ([#6326](https://github.com/PyTorchLightning/pytorch-lightning/pull/6326)
+- Ensure that clip gradients is only called if the value is greater than 0 ([#6330](https://github.com/PyTorchLightning/pytorch-lightning/pull/6330)
+- Fixed `Trainer` not resetting `lightning_optimizers` when calling `Trainer.fit()` multiple times ([#6372](https://github.com/PyTorchLightning/pytorch-lightning/pull/6372))
 
 
-- Fixed `Trainer` not resetting `lightning_optimizers` when calling `Trainer.fit()` multiple times ([#6372](https://github.com/PyTorchLightning/pytorch-lightning/pull/6372))
+- Fixed `DummyLogger.log_hyperparams` raising a `TypeError` when running with `fast_dev_run=True` ([#6398](https://github.com/PyTorchLightning/pytorch-lightning/pull/6398))
 
 
 ## [1.2.2] - 2021-03-02

docs/source/advanced/multi_gpu.rst

@@ -332,7 +332,6 @@ There are cases in which it is NOT possible to use DDP. Examples are:
 
 - Jupyter Notebook, Google COLAB, Kaggle, etc.
 - You have a nested script without a root package
-- Your script needs to invoke both `.fit` and `.test`, or one of them multiple times
 
 In these situations you should use `dp` or `ddp_spawn` instead.
 

pytorch_lightning/loggers/base.py

@@ -279,12 +279,14 @@ def _sanitize_params(params: Dict[str, Any]) -> Dict[str, Any]:
         return params
 
     @abstractmethod
-    def log_hyperparams(self, params: argparse.Namespace):
+    def log_hyperparams(self, params: argparse.Namespace, *args, **kwargs):
         """
         Record hyperparameters.
 
         Args:
             params: :class:`~argparse.Namespace` containing the hyperparameters
+            args: Optional positional arguments, depends on the specific logger being used
+            kwargs: Optional keywoard arguments, depends on the specific logger being used
         """
 
     def log_graph(self, model: LightningModule, input_array=None) -> None:
@@ -418,41 +420,41 @@ def nop(*args, **kw):
     def __getattr__(self, _):
         return self.nop
 
-    def __getitem__(self, idx):
-        # enables self.logger[0].experiment.add_image
-        # and self.logger.experiment[0].add_image(...)
+    def __getitem__(self, idx) -> "DummyExperiment":
+        # enables self.logger.experiment[0].add_image(...)
         return self
 
 
 class DummyLogger(LightningLoggerBase):
-    """ Dummy logger for internal use. Is usefull if we want to disable users
-        logger for a feature, but still secure that users code can run """
+    """
+    Dummy logger for internal use. It is useful if we want to disable user's
+    logger for a feature, but still ensure that user code can run
+    """
 
     def __init__(self):
         super().__init__()
         self._experiment = DummyExperiment()
 
     @property
-    def experiment(self):
+    def experiment(self) -> DummyExperiment:
         return self._experiment
 
-    @rank_zero_only
-    def log_metrics(self, metrics, step):
+    def log_metrics(self, *args, **kwargs) -> None:
         pass
 
-    @rank_zero_only
-    def log_hyperparams(self, params):
+    def log_hyperparams(self, *args, **kwargs) -> None:
         pass
 
     @property
-    def name(self):
-        pass
+    def name(self) -> str:
+        return ""
 
     @property
-    def version(self):
-        pass
+    def version(self) -> str:
+        return ""
 
-    def __getitem__(self, idx):
+    def __getitem__(self, idx) -> "DummyLogger":
+        # enables self.logger[0].experiment.add_image(...)
         return self
 
 

pytorch_lightning/trainer/trainer.py

@@ -381,21 +381,6 @@ def __init__(
         # Callback system
         self.on_init_end()
 
-    def setup_trainer(self, model: LightningModule):
-        """
-        Sanity check a few things before starting actual training or testing.
-
-        Args:
-            model: The model to run sanity test on.
-        """
-
-        # log hyper-parameters
-        if self.logger is not None:
-            # save exp to get started (this is where the first experiment logs are written)
-            self.logger.log_hyperparams(model.hparams_initial)
-            self.logger.log_graph(model)
-            self.logger.save()
-
     def fit(
         self,
         model: LightningModule,
@@ -444,7 +429,6 @@ def fit(
         self.call_setup_hook(model)
         self.call_hook("on_before_accelerator_backend_setup", model)
         self.accelerator.setup(self, model)  # note: this sets up self.lightning_module
-        self.setup_trainer(model)
 
         # ----------------------------
         # INSPECT THE CORE LOOPS
@@ -509,6 +493,13 @@ def fit(
     def pre_dispatch(self):
         self.accelerator.pre_dispatch()
 
+        # log hyper-parameters
+        if self.logger is not None:
+            # save exp to get started (this is where the first experiment logs are written)
+            self.logger.log_hyperparams(self.lightning_module.hparams_initial)
+            self.logger.log_graph(self.lightning_module)
+            self.logger.save()
+
     def post_dispatch(self):
         self.accelerator.post_dispatch()
         self.accelerator.teardown()

pytorch_lightning/tuner/lr_finder.py

@@ -418,11 +418,11 @@ def on_train_batch_end(self, trainer, pl_module, outputs, batch, batch_idx, data
             self.progress_bar.update()
 
         current_loss = trainer.train_loop.running_loss.last().item()
-        current_step = trainer.global_step + 1  # remove the +1 in 1.0
+        current_step = trainer.global_step
 
         # Avg loss (loss with momentum) + smoothing
         self.avg_loss = self.beta * self.avg_loss + (1 - self.beta) * current_loss
-        smoothed_loss = self.avg_loss / (1 - self.beta**current_step)
+        smoothed_loss = self.avg_loss / (1 - self.beta**(current_step + 1))
 
         # Check if we diverging
         if self.early_stop_threshold is not None:

tests/accelerators/test_accelerator_connector.py

@@ -13,6 +13,7 @@
 # limitations under the License
 
 import os
+from typing import Optional
 from unittest import mock
 
 import pytest
@@ -30,6 +31,7 @@
     DDPSpawnPlugin,
     DDPSpawnShardedPlugin,
     DeepSpeedPlugin,
+    ParallelPlugin,
     PrecisionPlugin,
     SingleDevicePlugin,
 )
@@ -408,10 +410,8 @@ def test_ipython_incompatible_backend_error(*_):
     ["accelerator", "plugin"],
     [('ddp_spawn', 'ddp_sharded'), (None, 'ddp_sharded')],
 )
-def test_plugin_accelerator_choice(accelerator, plugin):
-    """
-    Ensure that when a plugin and accelerator is passed in, that the plugin takes precedent.
-    """
+def test_plugin_accelerator_choice(accelerator: Optional[str], plugin: str):
+    """Ensure that when a plugin and accelerator is passed in, that the plugin takes precedent."""
     trainer = Trainer(accelerator=accelerator, plugins=plugin, num_processes=2)
     assert isinstance(trainer.accelerator.training_type_plugin, DDPShardedPlugin)
 
@@ -428,7 +428,9 @@ def test_plugin_accelerator_choice(accelerator, plugin):
 ])
 @mock.patch('torch.cuda.is_available', return_value=True)
 @mock.patch('torch.cuda.device_count', return_value=2)
-def test_accelerator_choice_multi_node_gpu(mock_is_available, mock_device_count, accelerator, plugin, tmpdir):
+def test_accelerator_choice_multi_node_gpu(
+    mock_is_available, mock_device_count, tmpdir, accelerator: str, plugin: ParallelPlugin
+):
     trainer = Trainer(
         accelerator=accelerator,
         default_root_dir=tmpdir,

tests/callbacks/test_callback_hook_outputs.py

@@ -18,7 +18,7 @@
 
 
 @pytest.mark.parametrize("single_cb", [False, True])
-def test_train_step_no_return(tmpdir, single_cb):
+def test_train_step_no_return(tmpdir, single_cb: bool):
     """
     Tests that only training_step can be used
     """

tests/callbacks/test_early_stopping.py

@@ -14,6 +14,7 @@
 import logging
 import os
 import pickle
+from typing import List, Optional
 from unittest import mock
 
 import cloudpickle
@@ -119,7 +120,7 @@ def test_early_stopping_no_extraneous_invocations(tmpdir):
         ([6, 5, 6, 5, 5, 5], 3, 4),
     ],
 )
-def test_early_stopping_patience(tmpdir, loss_values, patience, expected_stop_epoch):
+def test_early_stopping_patience(tmpdir, loss_values: list, patience: int, expected_stop_epoch: int):
     """Test to ensure that early stopping is not triggered before patience is exhausted."""
 
     class ModelOverrideValidationReturn(BoringModel):
@@ -142,7 +143,7 @@ def validation_epoch_end(self, outputs):
     assert trainer.current_epoch == expected_stop_epoch
 
 
-@pytest.mark.parametrize('validation_step', ['base', None])
+@pytest.mark.parametrize('validation_step_none', [True, False])
 @pytest.mark.parametrize(
     "loss_values, patience, expected_stop_epoch",
     [
@@ -151,7 +152,9 @@ def validation_epoch_end(self, outputs):
         ([6, 5, 6, 5, 5, 5], 3, 4),
     ],
 )
-def test_early_stopping_patience_train(tmpdir, validation_step, loss_values, patience, expected_stop_epoch):
+def test_early_stopping_patience_train(
+    tmpdir, validation_step_none: bool, loss_values: list, patience: int, expected_stop_epoch: int
+):
     """Test to ensure that early stopping is not triggered before patience is exhausted."""
 
     class ModelOverrideTrainReturn(BoringModel):
@@ -163,7 +166,7 @@ def training_epoch_end(self, outputs):
 
     model = ModelOverrideTrainReturn()
 
-    if validation_step is None:
+    if validation_step_none:
         model.validation_step = None
 
     early_stop_callback = EarlyStopping(monitor="train_loss", patience=patience, verbose=True)
@@ -254,7 +257,7 @@ def validation_epoch_end(self, outputs):
 
 
 @pytest.mark.parametrize('step_freeze, min_steps, min_epochs', [(5, 1, 1), (5, 1, 3), (3, 15, 1)])
-def test_min_steps_override_early_stopping_functionality(tmpdir, step_freeze, min_steps, min_epochs):
+def test_min_steps_override_early_stopping_functionality(tmpdir, step_freeze: int, min_steps: int, min_epochs: int):
     """Excepted Behaviour:
     IF `min_steps` was set to a higher value than the `trainer.global_step` when `early_stopping` is being triggered,
     THEN the trainer should continue until reaching `trainer.global_step` == `min_steps`, and stop.
@@ -386,10 +389,10 @@ def on_train_end(self) -> None:
                      marks=RunIf(skip_windows=True)),
     ],
 )
-def test_multiple_early_stopping_callbacks(callbacks, expected_stop_epoch, accelerator, num_processes, tmpdir):
-    """
-    Ensure when using multiple early stopping callbacks we stop if any signals we should stop.
-    """
+def test_multiple_early_stopping_callbacks(
+    tmpdir, callbacks: List[EarlyStopping], expected_stop_epoch: int, accelerator: Optional[str], num_processes: int
+):
+    """Ensure when using multiple early stopping callbacks we stop if any signals we should stop."""
 
     model = EarlyStoppingModel(expected_stop_epoch)
 

tests/callbacks/test_lr_monitor.py

@@ -51,10 +51,8 @@ def test_lr_monitor_single_lr(tmpdir):
 
 
 @pytest.mark.parametrize('opt', ['SGD', 'Adam'])
-def test_lr_monitor_single_lr_with_momentum(tmpdir, opt):
-    """
-    Test that learning rates and momentum are extracted and logged for single lr scheduler.
-    """
+def test_lr_monitor_single_lr_with_momentum(tmpdir, opt: str):
+    """Test that learning rates and momentum are extracted and logged for single lr scheduler."""
 
     class LogMomentumModel(BoringModel):
 
@@ -170,7 +168,7 @@ def test_lr_monitor_no_logger(tmpdir):
 
 
 @pytest.mark.parametrize("logging_interval", ['step', 'epoch'])
-def test_lr_monitor_multi_lrs(tmpdir, logging_interval):
+def test_lr_monitor_multi_lrs(tmpdir, logging_interval: str):
     """ Test that learning rates are extracted and logged for multi lr schedulers. """
     tutils.reset_seed()