Add missing test for "multiple dataloader + percent_check fix"

CHANGELOG.md

@@ -88,7 +88,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Fixed an issue with `_auto_collect_arguments` collecting local variables that are not constructor arguments and not working for signatures that have the instance not named `self` ([#2048](https://github.com/PyTorchLightning/pytorch-lightning/pull/2048))
 - Fixed mistake in parameters' grad norm tracking ([#2012](https://github.com/PyTorchLightning/pytorch-lightning/pull/2012))
 - Fixed CPU and hanging GPU crash ([#2118](https://github.com/PyTorchLightning/pytorch-lightning/pull/2118))
-
+- Fixed number batches in case of multiple dataloaders and `limit_{*}_batches` ([#1920](https://github.com/PyTorchLightning/pytorch-lightning/pull/1920), [#2226](https://github.com/PyTorchLightning/pytorch-lightning/pull/2226))
 - Fixed an issue with the model summary and `example_input_array` depending on a specific ordering of the submodules in a LightningModule ([#1773](https://github.com/PyTorchLightning/pytorch-lightning/pull/1773))
 
 ## [0.7.6] - 2020-05-16

pytorch_lightning/trainer/training_loop.py

@@ -205,7 +205,7 @@ class TrainerTrainLoopMixin(ABC):
     check_val_every_n_epoch: ...
     num_training_batches: int
     val_check_batch: ...
-    num_val_batches: int
+    num_val_batches: List[int]
     disable_validation: bool
     fast_dev_run: ...
     accumulation_scheduler: ...

tests/base/model_test_dataloaders.py

@@ -6,7 +6,7 @@
 class TestDataloaderVariations(ABC):
 
     @abstractmethod
-    def dataloader(self, train: bool):
+    def dataloader(self, *args, **kwargs):
         """placeholder"""
 
     def test_dataloader(self):
@@ -15,6 +15,11 @@ def test_dataloader(self):
     def test_dataloader__infinite(self):
         return CustomInfDataloader(self.dataloader(train=False))
 
+    def test_dataloader__multiple_mixed_length(self):
+        lengths = [50, 30, 40]
+        dataloaders = [self.dataloader(train=False, num_samples=n) for n in lengths]
+        return dataloaders
+
     def test_dataloader__empty(self):
         return None
 

tests/base/model_test_epoch_ends.py

@@ -7,7 +7,7 @@ class TestEpochEndVariations(ABC):
 
     def test_epoch_end(self, outputs):
         """
-        Called at the end of validation to aggregate outputs
+        Called at the end of test epoch to aggregate outputs
         :param outputs: list of individual outputs of each validation step
         :return:
         """
@@ -40,7 +40,7 @@ def test_epoch_end(self, outputs):
 
     def test_epoch_end__multiple_dataloaders(self, outputs):
         """
-        Called at the end of validation to aggregate outputs
+        Called at the end of test epoch to aggregate outputs
         :param outputs: list of individual outputs of each validation step
         :return:
         """

tests/base/model_utilities.py

@@ -6,8 +6,8 @@
 class ModelTemplateData:
     hparams: ...
 
-    def dataloader(self, train):
-        dataset = TrialMNIST(root=self.data_root, train=train, download=True)
+    def dataloader(self, train: bool, num_samples: int = 100):
+        dataset = TrialMNIST(root=self.data_root, train=train, num_samples=num_samples, download=True)
 
         loader = DataLoader(
             dataset=dataset,

tests/base/model_valid_dataloaders.py

@@ -6,12 +6,17 @@
 class ValDataloaderVariations(ABC):
 
     @abstractmethod
-    def dataloader(self, train: bool):
+    def dataloader(self, *args, **kwargs):
         """placeholder"""
 
     def val_dataloader(self):
         return self.dataloader(train=False)
 
+    def val_dataloader__multiple_mixed_length(self):
+        lengths = [100, 30]
+        dataloaders = [self.dataloader(train=False, num_samples=n) for n in lengths]
+        return dataloaders
+
     def val_dataloader__multiple(self):
         return [self.dataloader(train=False),
                 self.dataloader(train=False)]

tests/base/model_valid_epoch_ends.py

@@ -28,7 +28,7 @@ def _mean(res, key):
         results = {'progress_bar': metrics_dict, 'log': metrics_dict}
         return results
 
-    def validation_epoch_end_multiple_dataloaders(self, outputs):
+    def validation_epoch_end__multiple_dataloaders(self, outputs):
         """
         Called at the end of validation to aggregate outputs
 

tests/trainer/test_dataloaders.py

@@ -89,7 +89,7 @@ def test_multiple_val_dataloader(tmpdir):
     model = EvalModelTemplate()
     model.val_dataloader = model.val_dataloader__multiple
     model.validation_step = model.validation_step__multiple_dataloaders
-    model.validation_epoch_end = model.validation_epoch_end_multiple_dataloaders
+    model.validation_epoch_end = model.validation_epoch_end__multiple_dataloaders
 
     # fit model
     trainer = Trainer(
@@ -224,7 +224,7 @@ def test_multiple_dataloaders_passed_to_fit(tmpdir, ckpt_path):
 
     model = EvalModelTemplate()
     model.validation_step = model.validation_step__multiple_dataloaders
-    model.validation_epoch_end = model.validation_epoch_end_multiple_dataloaders
+    model.validation_epoch_end = model.validation_epoch_end__multiple_dataloaders
     model.test_step = model.test_step__multiple_dataloaders
 
     # train, multiple val and multiple test passed to fit
@@ -251,6 +251,52 @@ def test_multiple_dataloaders_passed_to_fit(tmpdir, ckpt_path):
         f'Multiple `test_dataloaders` not initiated properly, got {trainer.test_dataloaders}'
 
 
+@pytest.mark.parametrize(
+    ['limit_train_batches', 'limit_val_batches', 'limit_test_batches'],
+    [
+        pytest.param(0.0, 0.0, 0.0),
+        pytest.param(0, 0, 0.5),
+        pytest.param(1.0, 1.0, 1.0),
+        pytest.param(0.2, 0.4, 0.4),
+    ]
+)
+def test_dataloaders_with_limit_batches_percent(tmpdir, limit_train_batches, limit_val_batches, limit_test_batches):
+    """Verify num_batches for val & test dataloaders passed with batch limit in percent"""
+    model = EvalModelTemplate()
+    model.val_dataloader = model.val_dataloader__multiple_mixed_length
+    model.test_dataloader = model.test_dataloader__multiple_mixed_length
+    model.validation_step = model.validation_step__multiple_dataloaders
+    model.validation_epoch_end = model.validation_epoch_end__multiple_dataloaders
+    model.test_step = model.test_step__multiple_dataloaders
+    model.test_epoch_end = model.test_epoch_end__multiple_dataloaders
+
+    # train, multiple val and multiple test passed with percent_check
+    trainer = Trainer(
+        default_root_dir=tmpdir,
+        max_epochs=1,
+        limit_train_batches=limit_train_batches,
+        limit_val_batches=limit_val_batches,
+        limit_test_batches=limit_test_batches,
+    )
+    trainer.fit(model)
+    expected_train_batches = int(len(trainer.train_dataloader) * limit_train_batches)
+    expected_val_batches = [
+        int(len(dataloader) * limit_val_batches) for dataloader in trainer.val_dataloaders
+    ]
+    assert trainer.num_training_batches == expected_train_batches, \
+        f'train_percent_check not working with train_dataloaders, got {trainer.num_training_batches}'
+
+    assert trainer.num_val_batches == expected_val_batches, \
+        f'val_percent_check not working with val_dataloaders, got {trainer.num_val_batches}'
+
+    trainer.test(ckpt_path=None)
+    expected_test_batches = [
+        int(len(dataloader) * limit_test_batches) for dataloader in trainer.test_dataloaders
+    ]
+    assert trainer.num_test_batches == expected_test_batches, \
+        f'test_percent_check not working with test_dataloaders, got {trainer.num_test_batches}'
+
+
 @pytest.mark.parametrize('ckpt_path', [None, 'best', 'specific'])
 def test_mixing_of_dataloader_options(tmpdir, ckpt_path):
     """Verify that dataloaders can be passed to fit"""