Repository: Lightning-AI/pytorch-lightning
Pull request: Avoid changing the current cudnn.benchmark value
## What does this PR do?

Re-land of https://github.com/PyTorchLightning/pytorch-lightning/pull/12020
Fixes https://github.com/PyTorchLightning/pytorch-lightning/issues/12713

### Does your PR introduce any breaking changes? If yes, please list them.

Benchmark now defaults to `False`, as it was in v1.5

## Before submitting

- [x] Was this **discussed/approved** via a GitHub issue? (not for typos and docs)
- [x] Did you read the [contributor guideline](https://github.com/PyTorchLightning/pytorch-lightning/blob/master/.github/CONTRIBUTING.md), **Pull Request** section?
- [x] Did you make sure your **PR does only one thing**, instead of bundling different changes together?
- [x] Did you make sure to **update the documentation** with your changes? (if necessary)
- [x] Did you write any **new necessary tests**? (not for typos and docs)
- [x] Did you verify new and **existing tests pass** locally with your changes?
- [x] Did you list all the **breaking changes** introduced by this pull request?
- [x] Did you **update the [CHANGELOG](https://github.com/PyTorchLightning/pytorch-lightning/blob/master/CHANGELOG.md)**? (not for typos, docs, test updates, or minor internal changes/refactors)

<!-- In the CHANGELOG, separate each item in the unreleased section by a blank line to reduce collisions -->

## PR review

- [x] Is this pull request ready for review? (if not, please submit in draft mode)
- [x] Check that all items from **Before submitting** are resolved
- [x] Make sure the title is self-explanatory and the description concisely explains the PR
- [x] Add labels and milestones (and optionally projects) to the PR so it can be classified


cc @justusschock @kaushikb11 @awaelchli @borda @rohitgr7 @akihironitta

Problem statement:
v1.6 is slower than v1.5
## 🐛 Bug

### 1.6.0 is 5 times slower than 1.5.0

I used the same code and parameters on version 1.5.0 and version 1.6.0.
1.6.0 is 5 times slower than 1.5.0.

### To Reproduce
#### 1.6.0
```
> pip install pytorch-lightning==1.6.0
...
> python run.py
{'optimizers': {'lr': 0.001, 'weight_decay': 0}, 'train': {'milestones': [20, 40, 60, 80, 100, 120, 160, 180, 200, 220], 'gamma': 0.5, 'batch_size': 16}, 'general': {'save_dir': 'logs'}, 'trainer': {'gpus': [0], 'accelerator': 'gpu', 'max_epochs': 1, 'val_check_interval': 1, 'limit_train_batches': 0.01, 'limit_val_batches': 0.01, 'profiler': 'simple'}, 'data': {'cls_type': 'cat'}, 'exp_name': 'test'}
Global seed set to 1234
GPU available: True, used: True
TPU available: False, using: 0 TPU cores
IPU available: False, using: 0 IPUs
HPU available: False, using: 0 HPUs
`Trainer(val_check_interval=1)` was configured so validation will run after every batch.
Missing logger folder: logs/lightning_logs
LOCAL_RANK: 0 - CUDA_VISIBLE_DEVICES: [0]

  | Name      | Type               | Params
-------------------------------------------------
0 | model     | PvnetModelResnet18 | 13.0 M
1 | vote_crit | SmoothL1Loss       | 0     
2 | seg_crit  | CrossEntropyLoss   | 0     
-------------------------------------------------
13.0 M    Trainable params
0         Non-trainable params
13.0 M    Total params
51.831    Total estimated model params size (MB)
loading annotations into memory...
Done (t=1.30s)
creating index...
index created!
Epoch 0: 100%|██████████████████████████████████████████████████████| 12/12 [00:25<00:00,  2.11s/it, loss=0.861, v_num=0, train_ver_loss=0.205, train_seg_loss=0.656]
FIT Profiler Report

------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|  Action                                                                                                                                                                                                            |  Mean duration (s)    |  Num calls            |  Total time (s)       |  Percentage %         |
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|  Total                                                                                                                                                                                                             |  -                    |  574                  |  29.691               |  100 %                |
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|  run_training_epoch                                                                                                                                                                                                |  25.136               |  1                    |  25.136               |  84.66                |
|  run_training_batch                                                                                                                                                                                                |  1.7876               |  12                   |  21.451               |  72.247               |
|  [LightningModule]LitPvnet.optimizer_step                                                                                                                                                                          |  1.787                |  12                   |  21.444               |  72.225               |
|  [Strategy]SingleDeviceStrategy.backward                                                                                                                                                                           |  1.1394               |  12                   |  13.673               |  46.05                |
|  [Strategy]SingleDeviceStrategy.training_step                                                                                                                                                                      |  0.6367               |  12                   |  7.6404               |  25.733               |
|  [Callback]ModelCheckpoint{'monitor': None, 'mode': 'min', 'every_n_train_steps': 0, 'every_n_epochs': 1, 'train_time_interval': None, 'save_on_train_epoch_end': True}.on_train_epoch_end                         |  0.23877              |  1                    |  0.23877              |  0.80419              |
|  on_train_batch_end                                                                                                                                                                                                |  0.0010752            |  12                   |  0.012903             |  0.043458             |
```
#### 1.5.0
```
> pip install pytorch-lightning==1.5.0
...
> python run.py
{'optimizers': {'lr': 0.001, 'weight_decay': 0}, 'train': {'milestones': [20, 40, 60, 80, 100, 120, 160, 180, 200, 220], 'gamma': 0.5, 'batch_size': 16}, 'general': {'save_dir': 'logs'}, 'trainer': {'gpus': [0], 'accelerator': 'gpu', 'max_epochs': 1, 'val_check_interval': 1, 'limit_train_batches': 0.01, 'limit_val_batches': 0.01, 'profiler': 'simple'}, 'data': {'cls_type': 'cat'}, 'exp_name': 'test'}
Global seed set to 1234
GPU available: True, used: True
TPU available: False, using: 0 TPU cores
IPU available: False, using: 0 IPUs
LOCAL_RANK: 0 - CUDA_VISIBLE_DEVICES: [0]

  | Name      | Type               | Params
-------------------------------------------------
0 | model     | PvnetModelResnet18 | 13.0 M
1 | vote_crit | SmoothL1Loss       | 0     
2 | seg_crit  | CrossEntropyLoss   | 0     
-------------------------------------------------
13.0 M    Trainable params
0         Non-trainable params
13.0 M    Total params
51.831    Total estimated model params size (MB)
loading annotations into memory...
Done (t=1.37s)
creating index...
index created!
Epoch 0: 100%|██████████████████████████████████████████████████████| 12/12 [00:09<00:00,  1.23it/s, loss=0.861, v_num=0, train_ver_loss=0.205, train_seg_loss=0.656]
FIT Profiler Report

Action                                  |  Mean duration (s)    |Num calls              |  Total time (s)       |  Percentage %         |
--------------------------------------------------------------------------------------------------------------------------------------
Total                                   |  -                    |_                      |  14.094               |  100 %                |
--------------------------------------------------------------------------------------------------------------------------------------
run_training_epoch                      |  9.7956               |1                      |  9.7956               |  69.5                 |
run_training_batch                      |  0.44689              |12                     |  5.3626               |  38.048               |
get_train_batch                         |  0.23755              |13                     |  3.0882               |  21.911               |
fetch_next_train_batch                  |  0.23753              |13                     |  3.0879               |  21.909               |
optimizer_step_with_closure_0           |  0.2286               |12                     |  2.7432               |  19.463               |
training_step_and_backward              |  0.2224               |12                     |  2.6688               |  18.935               |
model_forward                           |  0.18607              |12                     |  2.2329               |  15.842               |
training_step                           |  0.18589              |12                     |  2.2306               |  15.827               |
backward                                |  0.035633             |12                     |  0.4276               |  3.0338               |
on_train_epoch_end                      |  0.4268               |1                      |  0.4268               |  3.0282               |
on_train_batch_end                      |  0.0063143            |12                     |  0.075772             |  0.53761              |
```

### Environment

- PyTorch Version: 1.8.1+cu111
- Python version: 3.8.13
- OS : Ubuntu
- CUDA/cuDNN version: 11.4
- How you installed PyTorch (`conda`, `pip`, source): conda


cc @borda @akihironitta

Hints from issue discussion:
I found that the loss received backward func on 1.6.0 was 2^16 times the loss returned by training_step func.
This is not the case in 1.5.0, the loss received backward func is equal to the loss returned by training_step func.
is it possible for you to share the script?
https://github.com/flyinghu123/pvnet-pl

> is it possible for you to share the script?


fyi @krshrimali 
@carmocca @krshrimali I've noticed this too and have attached a bunch of profiling and associated information here https://github.com/NVIDIA/NeMo/issues/4083

The performance downgrade from 1.5 to 1.6 is significant and definitely something that needs to be investigated!
Hi @piraka9011, thank you for the detailed information provided in the linked issue.

One possible cause is that we enabled `Trainer(benchmark=True)` by default from v1.6. It can lead to slower training if the input size varies frequently, and it may also lead to more memory consumption depending on the algorithms.

refs:
- https://discuss.pytorch.org/t/what-does-torch-backends-cudnn-benchmark-do/5936
- https://discuss.pytorch.org/t/set-torch-backends-cudnn-benchmark-true-consumes-huge-amount-of-memory/131010

Could you try disabling it (`Trainer(benchmark=False)`) and check the performance again?

---

@flyinghu123 @piraka9011  Do you see the regression when running without GPUs, too?
> Could you try disabling it (Trainer(benchmark=False)) and check the performance again?

This resolves the performance issue, thank you for sharing this info.

> One possible cause is that we enabled Trainer(benchmark=True) by default from v1.6.

Why? Can you please explain the reasoning behind this?

You are enabling a feature that has the potential to cause a performance slowdown and don't log any warnings or specify it in at least the release notes? (I couldn't find anything when I ctrl+f `benchmark`).
I did a bunch of profiling and debugging only to now realize the issue was just setting an arg.
And I'm pretty confident most lightning users won't realize this until they do their own (unnecessary) investigation.

You're already logging a bunch of deprecation notices, GPU usage, AMP, etc.
I propose that at the _very least_:
1. Add this info about potential slowdown to the docs (with ref to the PyTorch forum).
2. The lightning `Trainer` should be logging that "`benchmark` is enabled and that it may incur a performance slowdown if the model's input shapes change" (which is the case for audio/time-series tasks).

The better approach is to set this default to false and/or intelligently suggest to enable it if the tensor shapes aren't changing.
Hi @piraka9011! Sorry for the frustration caused.

We did not foresee the problems that could be caused by setting this flag by default. This was implemented in https://github.com/PyTorchLightning/pytorch-lightning/pull/11944 and mentioned in the "Changed" section of the release.

I am leaning towards defaulting to what torch defaults to which is False, meaning, reverting this change.

Does anybody from @PyTorchLightning/core-lightning object to this? This "fix" can be done by reviving this PR: https://github.com/PyTorchLightning/pytorch-lightning/pull/12020

Commit message: Avoid changing current cudnn benchmark

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cbc513279f27a..cfbf076e4e189 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -97,7 +97,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Changed `pytorch_lightning.core.lightning` to `pytorch_lightning.core.module` ([#12740](https://github.com/PyTorchLightning/pytorch-lightning/pull/12740))
 
 
--
+- Keep `torch.backends.cudnn.benchmark=False` by default (unlike in v1.6.{0-4}) after speed and memory problems depending on the data used. Please consider tuning `Trainer(benchmark)` manually. ([#13154](https://github.com/PyTorchLightning/pytorch-lightning/pull/13154))
+
+
+- Prevent modification of `torch.backends.cudnn.benchmark` when `Trainer(benchmark=...)` is not set ([#13154](https://github.com/PyTorchLightning/pytorch-lightning/pull/13154))
 
 ### Deprecated
 
diff --git a/docs/source/common/trainer.rst b/docs/source/common/trainer.rst
index 253dbb41b0e06..ae327ef4ad222 100644
--- a/docs/source/common/trainer.rst
+++ b/docs/source/common/trainer.rst
@@ -437,21 +437,24 @@ benchmark
 
 |
 
-Defaults to ``True`` if :paramref:`~pytorch_lightning.trainer.Trainer.deterministic` is not set.
-This flag sets the ``torch.backends.cudnn.benchmark`` flag. You can read more about its impact
+The value (``True`` or ``False``) to set ``torch.backends.cudnn.benchmark`` to. The value for
+``torch.backends.cudnn.benchmark`` set in the current session will be used (``False`` if not manually set).
+If :paramref:`~pytorch_lightning.trainer.Trainer.deterministic` is set to ``True``, this will default to ``False``.
+You can read more about the interaction of ``torch.backends.cudnn.benchmark`` and ``torch.backends.cudnn.deterministic``
 `here <https://pytorch.org/docs/stable/notes/randomness.html#cuda-convolution-benchmarking>`__
 
-This is likely to increase the speed of your system if your input sizes don't change. However, if they do, then it
-might make your system slower. The CUDNN auto-tuner will try to find the best algorithm for the hardware when a new
-input size is encountered. Read more about it `here <https://discuss.pytorch.org/t/what-does-torch-backends-cudnn-benchmark-do/5936>`__.
+Setting this flag to ``True`` can increase the speed of your system if your input sizes don't
+change. However, if they do, then it might make your system slower. The CUDNN auto-tuner will try to find the best
+algorithm for the hardware when a new input size is encountered. This might also increase the memory usage.
+Read more about it `here <https://discuss.pytorch.org/t/what-does-torch-backends-cudnn-benchmark-do/5936>`__.
 
 Example::
 
-    # defaults to True if not deterministic (which is False by default)
-    trainer = Trainer()
+    # Will use whatever the current value for torch.backends.cudnn.benchmark, normally False
+    trainer = Trainer(benchmark=None)  # default
 
     # you can overwrite the value
-    trainer = Trainer(benchmark=False)
+    trainer = Trainer(benchmark=True)
 
 deterministic
 ^^^^^^^^^^^^^
diff --git a/pytorch_lightning/trainer/connectors/accelerator_connector.py b/pytorch_lightning/trainer/connectors/accelerator_connector.py
index 753234bc21d83..f135237c13503 100644
--- a/pytorch_lightning/trainer/connectors/accelerator_connector.py
+++ b/pytorch_lightning/trainer/connectors/accelerator_connector.py
@@ -148,14 +148,19 @@ def __init__(
             A. Class > str
             B. Strategy > Accelerator/precision/plugins
         """
-        if benchmark and deterministic:
-            rank_zero_warn(
-                "You passed `deterministic=True` and `benchmark=True`. Note that PyTorch ignores"
-                " torch.backends.cudnn.deterministic=True when torch.backends.cudnn.benchmark=True.",
-            )
-        self.benchmark = not deterministic if benchmark is None else benchmark
+        if deterministic:
+            if benchmark is None:
+                # Set benchmark to False to ensure determinism
+                benchmark = False
+            elif benchmark:
+                rank_zero_warn(
+                    "You passed `deterministic=True` and `benchmark=True`. Note that PyTorch ignores"
+                    " torch.backends.cudnn.deterministic=True when torch.backends.cudnn.benchmark=True.",
+                )
         # TODO: move to gpu accelerator
-        torch.backends.cudnn.benchmark = self.benchmark
+        if benchmark is not None:
+            torch.backends.cudnn.benchmark = benchmark
+        self.benchmark = torch.backends.cudnn.benchmark
         self.replace_sampler_ddp = replace_sampler_ddp
         self._init_deterministic(deterministic)
 
@@ -215,13 +220,13 @@ def __init__(
         # 6. Instantiate Strategy - Part 2
         self._lazy_init_strategy()
 
-    def _init_deterministic(self, deterministic: Union[bool, _LITERAL_WARN]) -> None:
-        self.deterministic = deterministic
+    def _init_deterministic(self, deterministic: Optional[Union[bool, _LITERAL_WARN]]) -> None:
+        self.deterministic = deterministic or False  # default to False if not set
         if _TORCH_GREATER_EQUAL_1_11 and deterministic == "warn":
             torch.use_deterministic_algorithms(True, warn_only=True)
         else:
-            torch.use_deterministic_algorithms(deterministic)
-        if deterministic:
+            torch.use_deterministic_algorithms(self.deterministic)
+        if self.deterministic:
             # fixing non-deterministic part of horovod
             # https://github.com/PyTorchLightning/pytorch-lightning/pull/1572/files#r420279383
             os.environ["HOROVOD_FUSION_THRESHOLD"] = "0"
diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py
index d9dc550f10830..be7edeb5099e5 100644
--- a/pytorch_lightning/trainer/trainer.py
+++ b/pytorch_lightning/trainer/trainer.py
@@ -172,7 +172,7 @@ def __init__(
         resume_from_checkpoint: Optional[Union[Path, str]] = None,
         profiler: Optional[Union[Profiler, str]] = None,
         benchmark: Optional[bool] = None,
-        deterministic: Union[bool, _LITERAL_WARN] = False,
+        deterministic: Optional[Union[bool, _LITERAL_WARN]] = None,
         reload_dataloaders_every_n_epochs: int = 0,
         auto_lr_find: Union[bool, str] = False,
         replace_sampler_ddp: bool = True,
@@ -224,9 +224,11 @@ def __init__(
                 that only one process at a time can access them.
                 Default: ``False``.
 
-            benchmark: Sets ``torch.backends.cudnn.benchmark``.
-                Defaults to ``True`` if :paramref:`~pytorch_lightning.trainer.trainer.Trainer.deterministic`
-                is ``False``. Overwrite to manually set a different value. Default: ``None``.
+            benchmark: The value (``True`` or ``False``) to set ``torch.backends.cudnn.benchmark`` to.
+                The value for ``torch.backends.cudnn.benchmark`` set in the current session will be used
+                (``False`` if not manually set). If :paramref:`~pytorch_lightning.trainer.Trainer.deterministic` is set
+                to ``True``, this will default to ``False``. Override to manually set a different value.
+                Default: ``None``.
 
             callbacks: Add a callback or list of callbacks.
                 Default: ``None``.
@@ -250,8 +252,8 @@ def __init__(
 
             deterministic: If ``True``, sets whether PyTorch operations must use deterministic algorithms.
                 Set to ``"warn"`` to use deterministic algorithms whenever possible, throwing warnings on operations
-                that don't support deterministic mode (requires Pytorch 1.11+).
-                Default: ``False``.
+                that don't support deterministic mode (requires Pytorch 1.11+). If not set, defaults to ``False``.
+                Default: ``None``.
 
             devices: Will be mapped to either `gpus`, `tpu_cores`, `num_processes` or `ipus`,
                 based on the accelerator type.
diff --git a/tests/trainer/test_trainer.py b/tests/trainer/test_trainer.py
index 81f3bf88240a8..8a834825371b5 100644
--- a/tests/trainer/test_trainer.py
+++ b/tests/trainer/test_trainer.py
@@ -642,12 +642,15 @@ def test_trainer_max_steps_accumulate_batches(tmpdir):
 @pytest.mark.parametrize(
     ["benchmark_", "deterministic", "expected"],
     [
-        (None, False, True),
+        (None, False, None),
         (None, True, False),
+        (None, None, None),
         (True, False, True),
         (True, True, True),
-        (False, True, False),
+        (True, None, True),
         (False, False, False),
+        (False, True, False),
+        (False, None, False),
     ],
 )
 def test_benchmark_option(benchmark_, deterministic, expected):
@@ -660,6 +663,7 @@ def test_benchmark_option(benchmark_, deterministic, expected):
             trainer = Trainer(benchmark=benchmark_, deterministic=deterministic)
     else:
         trainer = Trainer(benchmark=benchmark_, deterministic=deterministic)
+    expected = original_val if expected is None else expected
     assert torch.backends.cudnn.benchmark == expected
     assert trainer._accelerator_connector.benchmark == expected
 


Code review comments:
diff_hunk: @@ -642,12 +642,15 @@ def test_trainer_max_steps_accumulate_batches(tmpdir):
 @pytest.mark.parametrize(
     ["benchmark_", "deterministic", "expected"],
     [
-        (None, False, True),
+        (None, False, None),
         (None, True, False),
+        (None, None, None),
         (True, False, True),
         (True, True, True),
-        (False, True, False),
+        (True, None, True),
         (False, False, False),
+        (False, True, False),
+        (False, None, False),
     ],
 )
 def test_benchmark_option(benchmark_, deterministic, expected):
path: tests/trainer/test_trainer.py
text: @user1:
we don't have a test where we set both values for`torch.backends.cudnn.benchmark`, do we?
The test below always has `original_val=True` if I'm not mistaken

@author:
Added