Repository: Lightning-AI/pytorch-lightning
Pull request: Enables reload of dataloaders on every n epochs from every epoch
## What does this PR do?
This PR extends the logic of reloading dataloaders on every epoch allowing DLs to be reloaded every n epochs. It changes the argument from `reload_dataloaders_every_epoch ` to `reload_dataloaders_every_n_epochs `.  The parameter accepts an `int`>=  0. The default value is 0 

Fixes #5001 

## Before submitting
- [x] Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
- [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? Otherwise, we ask you to create a separate PR for every change.
- [x] Did you make sure to update the documentation with your changes?
- [x] Did you write any new necessary tests? 
- [x] Did you verify new and existing tests pass locally with your changes?
- [x] If you made a notable change (that affects users), did you update the [CHANGELOG](https://github.com/PyTorchLightning/pytorch-lightning/blob/master/CHANGELOG.md)?

<!-- For CHANGELOG separate each item in the unreleased section by a blank line to reduce collisions -->

## PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read [Review guidelines](https://github.com/PyTorchLightning/pytorch-lightning/wiki/Review-guidelines). In short, see the following bullet-list:  

 - [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; _Bugfixes should be including in bug-fix release milestones (m.f.X) and features should be included in (m.X.b) releases._
 

## Did you have fun?
Make sure you had fun coding 🙃


Problem statement:
[Enhance] Update reload_dataloaders_every_epoch to reload_dataloaders_every_n_epoch
## 🚀 Feature
`reload_dataloaders_every_epoch` reloads dataloader every epoch. We can change it to reload dataloaders after every n epochs. It will accept both bool and int values. `True` will be treated as 1 and `False` will be treated as 0 (means no reload).

### Motivation
Recently a request came up on slack regarding reloading dataloaders after some epochs. Also since we have enhanced `fast_dev_run` like this, we can update this too.

### Alternatives
Can be done manually by calling the method to reload dataloaders with a simple conditional statement by setting `reload_dataloaders_every_epoch=False`, but we can let this attribute accept an integer to avoid manual work.

Hints from issue discussion:
cc @PyTorchLightning/core-contributors is this a good feature to have??
This issue has been automatically marked as stale because it hasn't had any recent activity. This issue will be closed in 7 days if no further activity occurs. Thank you for your contributions, Pytorch Lightning Team!

Commit message: Fix CHANGELOG. Update deprecation versions

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 114f1af38f82c..b951a593e49e3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -54,6 +54,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 - MLflowLogger now uses the env variable `MLFLOW_TRACKING_URI` as default tracking uri ([#7457](https://github.com/PyTorchLightning/pytorch-lightning/pull/7457))
 
+- Changed `Trainer` arg and functionality from `reload_dataloaders_every_epoch` to `reload_dataloaders_every_n_epochs` ([#5043](https://github.com/PyTorchLightning/pytorch-lightning/pull/5043))
 
 ### Deprecated
 
@@ -64,6 +65,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Deprecated `num_nodes` and `sync_batchnorm` arguments in `DDPPlugin` and `DDPSpawnPlugin` ([7026](https://github.com/PyTorchLightning/pytorch-lightning/pull/7026))
 
 
+- Deprecated `reload_dataloaders_every_epoch` argument of `Trainer` ([#5043](https://github.com/PyTorchLightning/pytorch-lightning/pull/5043))
+
+
 ### Removed
 
 - Prune deprecated classif. metrics from `pytorch_lightning.metrics.functional.classification` ([7499](https://github.com/PyTorchLightning/pytorch-lightning/pull/7499))
@@ -332,6 +336,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
     [#6323](https://github.com/PyTorchLightning/pytorch-lightning/pull/6323),
     [#6211](https://github.com/PyTorchLightning/pytorch-lightning/pull/6211))
 
+
 ## [1.2.9] - 2021-04-20
 
 ### Fixed
@@ -376,8 +381,6 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Enforce an epoch scheduler interval when using SWA ([#6588](https://github.com/PyTorchLightning/pytorch-lightning/pull/6588))
 - Fixed TPU Colab hang issue, post training ([#6816](https://github.com/PyTorchLightning/pytorch-lightning/pull/6816))
 - Fixed a bug where `TensorBoardLogger` would give a warning and not log correctly to a symbolic link `save_dir` ([#6730](https://github.com/PyTorchLightning/pytorch-lightning/pull/6730))
-
-
 - Fixed bug where `predict` could not be used when `progress_bar_refresh_rate=0` ([#6884](https://github.com/PyTorchLightning/pytorch-lightning/pull/6884))
 
 
diff --git a/docs/source/common/trainer.rst b/docs/source/common/trainer.rst
index f9275fcbd898b..49e97975642db 100644
--- a/docs/source/common/trainer.rst
+++ b/docs/source/common/trainer.rst
@@ -1252,8 +1252,8 @@ Note:
       Lightning will set it to 20 in these environments if the user does not provide a value.
     - This argument is ignored if a custom callback is passed to :paramref:`~Trainer.callbacks`.
 
-reload_dataloaders_every_epoch
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+reload_dataloaders_every_n_epochs
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 .. raw:: html
 
@@ -1263,19 +1263,20 @@ reload_dataloaders_every_epoch
 
 |
 
-Set to True to reload dataloaders every epoch.
+Set to a postive integer to reload dataloaders every n epochs.
 
 .. code-block:: python
 
-    # if False (default)
+    # if 0 (default)
     train_loader = model.train_dataloader()
     for epoch in epochs:
         for batch in train_loader:
             ...
 
-    # if True
+    # if a positive integer
     for epoch in epochs:
-        train_loader = model.train_dataloader()
+        if not epoch % reload_dataloaders_every_n_epochs:
+            train_loader = model.train_dataloader()
         for batch in train_loader:
 
 replace_sampler_ddp
diff --git a/pytorch_lightning/core/hooks.py b/pytorch_lightning/core/hooks.py
index 0ad6b131ad14b..17ce2fe38a20d 100644
--- a/pytorch_lightning/core/hooks.py
+++ b/pytorch_lightning/core/hooks.py
@@ -437,8 +437,9 @@ def train_dataloader(self) -> Union[DataLoader, List[DataLoader], Dict[str, Data
             (list, dict, nested lists and dicts). In the case of multiple dataloaders, please see
             this :ref:`page <multiple-training-dataloaders>`
 
-        The dataloader you return will not be called every epoch unless you set
-        :paramref:`~pytorch_lightning.trainer.Trainer.reload_dataloaders_every_epoch` to ``True``.
+        The dataloader you return will not be reloaded unless you set
+        :paramref:`~pytorch_lightning.trainer.Trainer.reload_dataloaders_every_n_epochs` to
+        a positive integer.
 
         For data processing use the following pattern:
 
@@ -507,8 +508,9 @@ def test_dataloader(self) -> Union[DataLoader, List[DataLoader]]:
         r"""
         Implement one or multiple PyTorch DataLoaders for testing.
 
-        The dataloader you return will not be called every epoch unless you set
-        :paramref:`~pytorch_lightning.trainer.Trainer.reload_dataloaders_every_epoch` to ``True``.
+        The dataloader you return will not be reloaded unless you set
+        :paramref:`~pytorch_lightning.trainer.Trainer.reload_dataloaders_every_n_epochs` to
+        a postive integer.
 
         For data processing use the following pattern:
 
@@ -567,8 +569,9 @@ def val_dataloader(self) -> Union[DataLoader, List[DataLoader]]:
         r"""
         Implement one or multiple PyTorch DataLoaders for validation.
 
-        The dataloader you return will not be called every epoch unless you set
-        :paramref:`~pytorch_lightning.trainer.Trainer.reload_dataloaders_every_epoch` to ``True``.
+        The dataloader you return will not be reloaded unless you set
+        :paramref:`~pytorch_lightning.trainer.Trainer.reload_dataloaders_every_n_epochs` to
+        a positive integer.
 
         It's recommended that all data downloads and preparation happen in :meth:`prepare_data`.
 
diff --git a/pytorch_lightning/trainer/connectors/data_connector.py b/pytorch_lightning/trainer/connectors/data_connector.py
index 9fb531f8eb67c..f1463b3ed8b43 100644
--- a/pytorch_lightning/trainer/connectors/data_connector.py
+++ b/pytorch_lightning/trainer/connectors/data_connector.py
@@ -18,6 +18,7 @@
 
 import pytorch_lightning as pl
 from pytorch_lightning.trainer.supporters import prefetch_iterator
+from pytorch_lightning.utilities import rank_zero_warn
 from pytorch_lightning.utilities.exceptions import MisconfigurationException
 from pytorch_lightning.utilities.model_helpers import is_overridden
 
@@ -28,7 +29,11 @@ def __init__(self, trainer):
         self.trainer = trainer
 
     def on_trainer_init(
-        self, check_val_every_n_epoch: int, reload_dataloaders_every_epoch: bool, prepare_data_per_node: bool
+        self,
+        check_val_every_n_epoch: int,
+        reload_dataloaders_every_n_epochs: int,
+        reload_dataloaders_every_epoch: bool,
+        prepare_data_per_node: bool,
     ) -> None:
         self.trainer.datamodule = None
         self.trainer.prepare_data_per_node = prepare_data_per_node
@@ -39,7 +44,21 @@ def on_trainer_init(
             )
 
         self.trainer.check_val_every_n_epoch = check_val_every_n_epoch
-        self.trainer.reload_dataloaders_every_epoch = reload_dataloaders_every_epoch
+
+        if reload_dataloaders_every_epoch:
+            reload_dataloaders_every_n_epochs = int(reload_dataloaders_every_epoch)
+            rank_zero_warn(
+                "`reload_dataloaders_every_epoch` is deprecated in v1.4 and will be removed in v1.6."
+                " Please use `reload_dataloaders_every_n_epochs` in Trainer.", DeprecationWarning
+            )
+
+        if not isinstance(reload_dataloaders_every_n_epochs, int) or (reload_dataloaders_every_n_epochs < 0):
+            raise MisconfigurationException(
+                "`reload_dataloaders_every_n_epochs` should be an int >= 0,"
+                f" got {reload_dataloaders_every_n_epochs}."
+            )
+
+        self.trainer.reload_dataloaders_every_n_epochs = reload_dataloaders_every_n_epochs
         self.trainer._is_data_prepared = False
 
     def get_profiled_train_dataloader(self, train_dataloader):
diff --git a/pytorch_lightning/trainer/evaluation_loop.py b/pytorch_lightning/trainer/evaluation_loop.py
index 426f26b8a2da7..5fcd7aa57f48e 100644
--- a/pytorch_lightning/trainer/evaluation_loop.py
+++ b/pytorch_lightning/trainer/evaluation_loop.py
@@ -60,7 +60,7 @@ def get_evaluation_dataloaders(self) -> Tuple[Optional[List[DataLoader]], List[U
             max_batches = self.trainer.num_test_batches
         else:
             # val
-            if self.trainer.val_dataloaders is None or self.trainer.reload_dataloaders_every_epoch:
+            if self.trainer.val_dataloaders is None or self.trainer.should_reload_dl_epoch:
                 self.trainer.reset_val_dataloader(model)
             if self.trainer.sanity_checking:
                 self.trainer.num_sanity_val_batches = [
diff --git a/pytorch_lightning/trainer/properties.py b/pytorch_lightning/trainer/properties.py
index ff12e5c6e9053..633cff58a08bb 100644
--- a/pytorch_lightning/trainer/properties.py
+++ b/pytorch_lightning/trainer/properties.py
@@ -55,6 +55,7 @@ class TrainerProperties(ABC):
     accelerator_connector: AcceleratorConnector
     callbacks: List[Callback]
     checkpoint_connector: CheckpointConnector
+    reload_dataloaders_every_n_epochs: int
     limit_val_batches: int
     logger: LightningLoggerBase
     logger_connector: LoggerConnector
@@ -254,6 +255,12 @@ def progress_bar_dict(self) -> dict:
         all_metrics.update(**logged_metrics)
         return all_metrics
 
+    @property
+    def should_reload_dl_epoch(self) -> bool:
+        """ Check if dataloader should be reloaded in the current epoch. """
+        n_epochs = self.reload_dataloaders_every_n_epochs
+        return n_epochs and (not self.current_epoch % n_epochs)
+
     @property
     def disable_validation(self) -> bool:
         """ Check if validation is disabled during training. """
diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py
index a9a431ddbba5e..843c8ba913e62 100644
--- a/pytorch_lightning/trainer/trainer.py
+++ b/pytorch_lightning/trainer/trainer.py
@@ -131,6 +131,7 @@ def __init__(
         profiler: Optional[Union[BaseProfiler, str]] = None,
         benchmark: bool = False,
         deterministic: bool = False,
+        reload_dataloaders_every_n_epochs: int = 0,
         reload_dataloaders_every_epoch: bool = False,
         auto_lr_find: Union[bool, str] = False,
         replace_sampler_ddp: bool = True,
@@ -259,8 +260,15 @@ def __init__(
             num_sanity_val_steps: Sanity check runs n validation batches before starting the training routine.
                 Set it to `-1` to run all batches in all validation dataloaders.
 
+            reload_dataloaders_every_n_epochs: Set to a non negative integer to reload dataloaders every n epochs.
+                Default: 0
+
             reload_dataloaders_every_epoch: Set to True to reload dataloaders every epoch.
 
+            .. warning::
+               ``reload_dataloaders_every_epoch`` has been deprecated in v1.4 and will be removed in v1.6.
+               Please use ``reload_dataloaders_every_n_epochs``.
+
             replace_sampler_ddp: Explicitly enables or disables sampler replacement. If not specified this
                 will toggled automatically when DDP is used. By default it will add ``shuffle=True`` for
                 train sampler and ``shuffle=False`` for val/test sampler. If you want to customize it,
@@ -365,7 +373,8 @@ def __init__(
 
         # init data flags
         self.data_connector.on_trainer_init(
-            check_val_every_n_epoch, reload_dataloaders_every_epoch, prepare_data_per_node
+            check_val_every_n_epoch, reload_dataloaders_every_n_epochs, reload_dataloaders_every_epoch,
+            prepare_data_per_node
         )
 
         # init training tricks
diff --git a/pytorch_lightning/trainer/training_loop.py b/pytorch_lightning/trainer/training_loop.py
index f7da8e929e865..388662bcbac9f 100644
--- a/pytorch_lightning/trainer/training_loop.py
+++ b/pytorch_lightning/trainer/training_loop.py
@@ -148,7 +148,7 @@ def on_train_epoch_start(self, epoch):
         model = self.trainer.lightning_module
 
         # reset train dataloader
-        if epoch != 0 and self.trainer.reload_dataloaders_every_epoch:
+        if epoch != 0 and self.trainer.should_reload_dl_epoch:
             self.trainer.reset_train_dataloader(model)
 
         # todo: specify the possible exception
diff --git a/tests/core/test_datamodules.py b/tests/core/test_datamodules.py
index e6500a15eeed1..96ee490c90c2f 100644
--- a/tests/core/test_datamodules.py
+++ b/tests/core/test_datamodules.py
@@ -430,9 +430,11 @@ def transfer_batch_to_device(self, batch, device, dataloader_idx):
     assert torch.allclose(batch_gpu.targets.cpu(), torch.ones(5, 1, dtype=torch.long) * 2)
 
 
-def test_dm_reload_dataloaders_every_epoch(tmpdir):
-    """Test datamodule, where trainer argument
-    reload_dataloaders_every_epoch is set to True/False"""
+def test_dm_reload_dataloaders_every_n_epochs(tmpdir):
+    """
+    Test datamodule, where trainer argument
+    reload_dataloaders_every_n_epochs is set to a non negative integer
+    """
 
     class CustomBoringDataModule(BoringDataModule):
 
@@ -457,9 +459,9 @@ def train_dataloader(self):
 
     trainer = Trainer(
         default_root_dir=tmpdir,
-        max_epochs=2,
-        limit_train_batches=0.01,
-        reload_dataloaders_every_epoch=True,
+        max_epochs=3,
+        limit_train_batches=2,
+        reload_dataloaders_every_n_epochs=2,
     )
     trainer.fit(model, dm)
 
diff --git a/tests/deprecated_api/test_remove_1-6.py b/tests/deprecated_api/test_remove_1-6.py
index 0df5bc38f8483..02e901523cb36 100644
--- a/tests/deprecated_api/test_remove_1-6.py
+++ b/tests/deprecated_api/test_remove_1-6.py
@@ -61,3 +61,25 @@ def test_v1_6_0_ddp_spawn_num_nodes():
 def test_v1_6_0_ddp_spawn_sync_batchnorm():
     with pytest.deprecated_call(match="Argument `sync_batchnorm` in `DDPPlugin` is deprecated in v1.4"):
         DDPSpawnPlugin(sync_batchnorm=False)
+
+
+def test_reload_dataloaders_every_epoch_remove_in_v1_4_0(tmpdir):
+
+    model = BoringModel()
+
+    with pytest.deprecated_call(match='will be removed in v1.6'):
+        trainer = Trainer(
+            default_root_dir=tmpdir,
+            limit_train_batches=0.3,
+            limit_val_batches=0.3,
+            reload_dataloaders_every_epoch=True,
+            max_epochs=3,
+        )
+    trainer.fit(model)
+    trainer.test()
+
+    # verify the sequence
+    calls = trainer.dev_debugger.dataloader_sequence_calls
+    expected_sequence = ['val_dataloader'] + ['train_dataloader', 'val_dataloader'] * 3 + ['test_dataloader']
+    for call, expected in zip(calls, expected_sequence):
+        assert call['name'] == expected
diff --git a/tests/trainer/test_dataloaders.py b/tests/trainer/test_dataloaders.py
index c2e5e1c24ac78..0d88257f5212f 100644
--- a/tests/trainer/test_dataloaders.py
+++ b/tests/trainer/test_dataloaders.py
@@ -1285,7 +1285,7 @@ def test_dataloaders_load_only_once_val_interval(tmpdir):
         limit_train_batches=10,
         limit_val_batches=10,
         val_check_interval=0.3,
-        reload_dataloaders_every_epoch=True,
+        reload_dataloaders_every_n_epochs=True,
         max_epochs=3,
     )
     trainer.fit(model)
@@ -1349,17 +1349,16 @@ def test_dataloaders_load_only_once_no_sanity_check(tmpdir):
         assert call['name'] == expected
 
 
-@mock.patch.dict(os.environ, {"PL_DEV_DEBUG": "1"})
-def test_dataloaders_load_every_epoch(tmpdir):
+@pytest.mark.parametrize("n", [1, 2])
+def test_dataloaders_load_every_n_epochs(tmpdir, n):
 
-    model = EvalModelTemplate()
+    model = BoringModel()
 
-    # logger file to get meta
     trainer = Trainer(
         default_root_dir=tmpdir,
         limit_train_batches=0.3,
         limit_val_batches=0.3,
-        reload_dataloaders_every_epoch=True,
+        reload_dataloaders_every_n_epochs=n,
         max_epochs=3,
     )
     trainer.fit(model)
@@ -1367,26 +1366,29 @@ def test_dataloaders_load_every_epoch(tmpdir):
 
     trainer.test()
 
-    assert len(trainer.dev_debugger.val_dataloader_calls) == 4
-    assert len(trainer.dev_debugger.train_dataloader_calls) == 3
-    assert len(trainer.dev_debugger.test_dataloader_calls) == 1
-
     # verify the sequence
     calls = trainer.dev_debugger.dataloader_sequence_calls
-    expected_sequence = [
-        'val_dataloader',
-        'train_dataloader',
-        'val_dataloader',
-        'train_dataloader',
-        'val_dataloader',
-        'train_dataloader',
-        'val_dataloader',
-        'test_dataloader',
-    ]
+    expected_sequence = ['val_dataloader']
+    if n == 1:
+        expected_sequence += ['train_dataloader', 'val_dataloader'] * 3
+    elif n == 2:
+        expected_sequence += ['train_dataloader', 'val_dataloader'] * 2
+    expected_sequence += ['test_dataloader']
+
     for call, expected in zip(calls, expected_sequence):
         assert call['name'] == expected
 
 
+@pytest.mark.parametrize("n", ['test', -1])
+def test_dataloaders_load_every_n_epochs_exception(tmpdir, n):
+
+    with pytest.raises(MisconfigurationException, match='should be an int >'):
+        Trainer(
+            default_root_dir=tmpdir,
+            reload_dataloaders_every_n_epochs=n,
+        )
+
+
 @mock.patch.dict(os.environ, {"PL_DEV_DEBUG": "1"})
 def test_dataloaders_load_every_epoch_no_sanity_check(tmpdir):
 
@@ -1407,7 +1409,7 @@ def validation_step(self, batch, batch_idx):
         limit_train_batches=0.3,
         limit_val_batches=0.3,
         num_sanity_val_steps=0,
-        reload_dataloaders_every_epoch=True,
+        reload_dataloaders_every_n_epochs=True,
         max_epochs=3,
         callbacks=[checkpoint_callback],
     )


Code review comments:
diff_hunk: @@ -254,6 +255,12 @@ def progress_bar_dict(self) -> dict:
         all_metrics.update(**logged_metrics)
         return all_metrics
 
+    @property
+    def should_reload_dl_epoch(self) -> bool:
path: pytorch_lightning/trainer/properties.py
text: @user1:
Do you see this as a property that users might want to rely on?

If not, I wouldn't add this logic here

@author:
Perhaps. What do the others think?  @user2 [mentioned](https://github.com/PyTorchLightning/pytorch-lightning/pull/5043#discussion_r539507110) in a review earlier if this logic could become a property of the trainer 

@user1:
It can be in the trainer (at least for now) but let's mark it as protected so we don't have to deprecate later

`should_reload_dl_epoch` -> `_should_reload_dl_epoch`

@author:
Sure