Remove error when test dataloader used in test (#1495)

* remove error when test dataloader used in test

* remove error when test dataloader used in test

* remove error when test dataloader used in test

* remove error when test dataloader used in test

* remove error when test dataloader used in test

* remove error when test dataloader used in test

* fix lost model reference

* remove error when test dataloader used in test

* fix lost model reference

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* added tests for warning

* fix lost model reference

* fix lost model reference

* added tests for warning

* added tests for warning

* refactoring

* refactoring

* fix imports

* refactoring

* fix imports

* refactoring

* fix tests

* fix mnist

* flake8

* review

Co-authored-by: J. Borovec <[email protected]>

Author: williamFalcon

CHANGELOG.md

@@ -111,6 +111,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ### Removed
 
+- Removed test for no test dataloader in .fit ([#1495](https://github.com/PyTorchLightning/pytorch-lightning/pull/1495))
 - Removed duplicated module `pytorch_lightning.utilities.arg_parse` for loading CLI arguments ([#1167](https://github.com/PyTorchLightning/pytorch-lightning/issues/1167))
 - Removed wandb logger's `finalize` method ([#1193](https://github.com/PyTorchLightning/pytorch-lightning/pull/1193))
 - Dropped `torchvision` dependency in tests and added own MNIST dataset class instead ([#986](https://github.com/PyTorchLightning/pytorch-lightning/issues/986))

benchmarks/test_trainer_parity.py

@@ -11,7 +11,7 @@
 import tests.base.utils as tutils
 
 from pytorch_lightning import Trainer, LightningModule
-from tests.base.datasets import TestingMNIST
+from tests.base.datasets import TrialMNIST
 
 
 class ParityMNIST(LightningModule):
@@ -42,10 +42,10 @@ def configure_optimizers(self):
         return torch.optim.Adam(self.parameters(), lr=0.02)
 
     def train_dataloader(self):
-        return DataLoader(TestingMNIST(train=True,
-                                       download=True,
-                                       num_samples=500,
-                                       digits=list(range(5))),
+        return DataLoader(TrialMNIST(train=True,
+                                     download=True,
+                                     num_samples=500,
+                                     digits=list(range(5))),
                           batch_size=128)
 
 
@@ -65,10 +65,11 @@ def test_pytorch_parity(tmpdir):
     for pl_out, pt_out in zip(lightning_outs, manual_outs):
         np.testing.assert_almost_equal(pl_out, pt_out, 5)
 
-    tutils.assert_speed_parity(pl_times, pt_times, num_epochs)
+    # the fist run initialize dataset (download & filter)
+    tutils.assert_speed_parity(pl_times[1:], pt_times[1:], num_epochs)
 
 
-def set_seed(seed):
+def _set_seed(seed):
     np.random.seed(seed)
     torch.manual_seed(seed)
     if torch.cuda.is_available():
@@ -88,7 +89,7 @@ def vanilla_loop(MODEL, num_runs=10, num_epochs=10):
 
         # set seed
         seed = i
-        set_seed(seed)
+        _set_seed(seed)
 
         # init model parts
         model = MODEL()
@@ -134,7 +135,7 @@ def lightning_loop(MODEL, num_runs=10, num_epochs=10):
 
         # set seed
         seed = i
-        set_seed(seed)
+        _set_seed(seed)
 
         # init model parts
         model = MODEL()

pytorch_lightning/trainer/evaluation_loop.py

@@ -418,10 +418,8 @@ def evaluation_forward(self, model, batch, batch_idx, dataloader_idx, test_mode:
         # make dataloader_idx arg in validation_step optional
         args = [batch, batch_idx]
 
-        if test_mode and len(self.test_dataloaders) > 1:
-            args.append(dataloader_idx)
-
-        elif not test_mode and len(self.val_dataloaders) > 1:
+        if (test_mode and len(self.test_dataloaders) > 1) \
+                or (not test_mode and len(self.val_dataloaders) > 1):
             args.append(dataloader_idx)
 
         # handle DP, DDP forward

pytorch_lightning/trainer/model_hooks.py

@@ -21,6 +21,8 @@ def is_overriden(self, method_name: str, model: LightningModule = None) -> bool:
             return False
 
         instance_attr = getattr(model, method_name)
+        if not instance_attr:
+            return False
         super_attr = getattr(super_object, method_name)
 
         # when code pointers are different, it was implemented

pytorch_lightning/trainer/trainer.py

@@ -939,11 +939,14 @@ def test(self, model: Optional[LightningModule] = None, test_dataloaders: Option
         self.testing = True
 
         if test_dataloaders is not None:
-            if model is not None:
+            if model:
                 self.__attach_dataloaders(model, test_dataloaders=test_dataloaders)
             else:
                 self.__attach_dataloaders(self.model, test_dataloaders=test_dataloaders)
 
+        # give proper warnings if user only passed in loader without hooks
+        self.check_testing_model_configuration(model if model else self.model)
+
         if model is not None:
             self.model = model
             self.fit(model)
@@ -1012,10 +1015,25 @@ def check_model_configuration(self, model: LightningModule):
                         'You have defined a `test_dataloader()` and have defined a `test_step()`, you may also want to'
                         ' define `test_epoch_end()` for accumulating stats.', RuntimeWarning
                     )
-        else:
-            if self.is_overriden('test_step', model):
-                raise MisconfigurationException('You have defined `test_step()`,'
-                                                ' but have not passed in a `test_dataloader()`.')
+
+    def check_testing_model_configuration(self, model: LightningModule):
+
+        has_test_step = self.is_overriden('test_step', model)
+        has_test_epoch_end = self.is_overriden('test_epoch_end', model)
+        gave_test_loader = hasattr(model, 'test_dataloader') and model.test_dataloader()
+
+        if gave_test_loader and not has_test_step:
+            raise MisconfigurationException('You passed in a `test_dataloader` but did not implement `test_step()`')
+
+        if has_test_step and not gave_test_loader:
+            raise MisconfigurationException('You defined `test_step()` but did not implement'
+                                            ' `test_dataloader` nor passed in `.fit(test_dataloaders`.')
+
+        if has_test_step and gave_test_loader and not has_test_epoch_end:
+            rank_zero_warn(
+                'You passed  in a `test_dataloader` and have defined a `test_step()`, you may also want to'
+                ' define `test_epoch_end()` for accumulating stats.', RuntimeWarning
+            )
 
 
 class _PatchDataLoader(object):

tests/base/__init__.py

@@ -3,6 +3,7 @@
 import torch
 
 from tests.base.models import TestModelBase, DictHparamsModel
+from tests.base.eval_model_template import EvalModelTemplate
 from tests.base.mixins import (
     LightEmptyTestStep,
     LightValidationStepMixin,

tests/base/datasets.py

@@ -49,7 +49,7 @@ class MNIST(Dataset):
     cache_folder_name = 'complete'
 
     def __init__(self, root: str = PATH_DATASETS, train: bool = True,
-                 normalize: tuple = (0.5, 1.0), download: bool = False):
+                 normalize: tuple = (0.5, 1.0), download: bool = True):
         super().__init__()
         self.root = root
         self.train = train  # training set or test set
@@ -111,7 +111,7 @@ def normalize_tensor(tensor: Tensor, mean: float = 0.0, std: float = 1.0) -> Ten
     return tensor
 
 
-class TestingMNIST(MNIST):
+class TrialMNIST(MNIST):
     """Constrain image dataset
 
     Args:
@@ -127,7 +127,7 @@ class TestingMNIST(MNIST):
         digits: list selected MNIST digits/classes
 
     Examples:
-        >>> dataset = TestingMNIST(download=True)
+        >>> dataset = TrialMNIST(download=True)
         >>> len(dataset)
         300
         >>> sorted(set([d.item() for d in dataset.targets]))
@@ -179,6 +179,8 @@ def prepare_data(self, download: bool) -> None:
             self._download(super().cached_folder_path)
 
         for fname in (self.TRAIN_FILE_NAME, self.TEST_FILE_NAME):
-            data, targets = torch.load(os.path.join(super().cached_folder_path, fname))
+            path_fname = os.path.join(super().cached_folder_path, fname)
+            assert os.path.isfile(path_fname), 'Missing cached file: %s' % path_fname
+            data, targets = torch.load(path_fname)
             data, targets = self._prepare_subset(data, targets, self.num_samples, self.digits)
             torch.save((data, targets), os.path.join(self.cached_folder_path, fname))

tests/base/debug.py

@@ -3,7 +3,7 @@
 from torch.utils.data import DataLoader
 
 import pytorch_lightning as pl
-from tests.base.datasets import TestingMNIST
+from tests.base.datasets import TrialMNIST
 
 
 # from test_models import assert_ok_test_acc, load_model, \
@@ -42,10 +42,10 @@ def configure_optimizers(self):
         return [torch.optim.Adam(self.parameters(), lr=0.02)]
 
     def train_dataloader(self):
-        return DataLoader(TestingMNIST(train=True, num_samples=100), batch_size=16)
+        return DataLoader(TrialMNIST(train=True, num_samples=100), batch_size=16)
 
     def val_dataloader(self):
-        return DataLoader(TestingMNIST(train=False, num_samples=50), batch_size=16)
+        return DataLoader(TrialMNIST(train=False, num_samples=50), batch_size=16)
 
     def test_dataloader(self):
-        return DataLoader(TestingMNIST(train=False, num_samples=50), batch_size=16)
+        return DataLoader(TrialMNIST(train=False, num_samples=50), batch_size=16)

tests/base/eval_model_optimizers.py

@@ -0,0 +1,61 @@
+from abc import ABC
+
+from torch import optim
+
+
+class ConfigureOptimizersPool(ABC):
+    def configure_optimizers(self):
+        """
+        return whatever optimizers we want here.
+        :return: list of optimizers
+        """
+        optimizer = optim.Adam(self.parameters(), lr=self.hparams.learning_rate)
+        return optimizer
+
+    def configure_optimizers_empty(self):
+        return None
+
+    def configure_optimizers_lbfgs(self):
+        """
+        return whatever optimizers we want here.
+        :return: list of optimizers
+        """
+        optimizer = optim.LBFGS(self.parameters(), lr=self.hparams.learning_rate)
+        return optimizer
+
+    def configure_optimizers_multiple_optimizers(self):
+        """
+        return whatever optimizers we want here.
+        :return: list of optimizers
+        """
+        # try no scheduler for this model (testing purposes)
+        optimizer1 = optim.Adam(self.parameters(), lr=self.hparams.learning_rate)
+        optimizer2 = optim.Adam(self.parameters(), lr=self.hparams.learning_rate)
+        return optimizer1, optimizer2
+
+    def configure_optimizers_single_scheduler(self):
+        optimizer = optim.Adam(self.parameters(), lr=self.hparams.learning_rate)
+        lr_scheduler = optim.lr_scheduler.StepLR(optimizer, 1, gamma=0.1)
+        return [optimizer], [lr_scheduler]
+
+    def configure_optimizers_multiple_schedulers(self):
+        optimizer1 = optim.Adam(self.parameters(), lr=self.hparams.learning_rate)
+        optimizer2 = optim.Adam(self.parameters(), lr=self.hparams.learning_rate)
+        lr_scheduler1 = optim.lr_scheduler.StepLR(optimizer1, 1, gamma=0.1)
+        lr_scheduler2 = optim.lr_scheduler.StepLR(optimizer2, 1, gamma=0.1)
+
+        return [optimizer1, optimizer2], [lr_scheduler1, lr_scheduler2]
+
+    def configure_optimizers_mixed_scheduling(self):
+        optimizer1 = optim.Adam(self.parameters(), lr=self.hparams.learning_rate)
+        optimizer2 = optim.Adam(self.parameters(), lr=self.hparams.learning_rate)
+        lr_scheduler1 = optim.lr_scheduler.StepLR(optimizer1, 4, gamma=0.1)
+        lr_scheduler2 = optim.lr_scheduler.StepLR(optimizer2, 1, gamma=0.1)
+
+        return [optimizer1, optimizer2], \
+            [{'scheduler': lr_scheduler1, 'interval': 'step'}, lr_scheduler2]
+
+    def configure_optimizers_reduce_lr_on_plateau(self):
+        optimizer = optim.Adam(self.parameters(), lr=self.hparams.learning_rate)
+        lr_scheduler = optim.lr_scheduler.ReduceLROnPlateau(optimizer)
+        return [optimizer], [lr_scheduler]

tests/base/eval_model_template.py

@@ -0,0 +1,80 @@
+import torch
+import torch.nn as nn
+import torch.nn.functional as F
+
+from tests.base.datasets import TrialMNIST
+from pytorch_lightning.core.lightning import LightningModule
+from tests.base.eval_model_optimizers import ConfigureOptimizersPool
+from tests.base.eval_model_test_dataloaders import TestDataloaderVariations
+from tests.base.eval_model_test_epoch_ends import TestEpochEndVariations
+from tests.base.eval_model_test_steps import TestStepVariations
+from tests.base.eval_model_train_dataloaders import TrainDataloaderVariations
+from tests.base.eval_model_train_steps import TrainingStepVariations
+from tests.base.eval_model_valid_dataloaders import ValDataloaderVariations
+from tests.base.eval_model_valid_epoch_ends import ValidationEpochEndVariations
+from tests.base.eval_model_valid_steps import ValidationStepVariations
+from tests.base.eval_model_utils import ModelTemplateUtils
+
+
+class EvalModelTemplate(
+    ModelTemplateUtils,
+    TrainingStepVariations,
+    ValidationStepVariations,
+    ValidationEpochEndVariations,
+    TestStepVariations,
+    TestEpochEndVariations,
+    TrainDataloaderVariations,
+    ValDataloaderVariations,
+    TestDataloaderVariations,
+    ConfigureOptimizersPool,
+    LightningModule
+):
+    """
+    This template houses all  combinations of model  configurations  we want to test
+    """
+    def __init__(self, hparams):
+        """Pass in parsed HyperOptArgumentParser to the model."""
+        # init superclass
+        super().__init__()
+        self.hparams = hparams
+
+        # if you specify an example input, the summary will show input/output for each layer
+        self.example_input_array = torch.rand(5, 28 * 28)
+
+        # build model
+        self.__build_model()
+
+    def __build_model(self):
+        """
+        Simple model for testing
+        :return:
+        """
+        self.c_d1 = nn.Linear(
+            in_features=self.hparams.in_features,
+            out_features=self.hparams.hidden_dim
+        )
+        self.c_d1_bn = nn.BatchNorm1d(self.hparams.hidden_dim)
+        self.c_d1_drop = nn.Dropout(self.hparams.drop_prob)
+
+        self.c_d2 = nn.Linear(
+            in_features=self.hparams.hidden_dim,
+            out_features=self.hparams.out_features
+        )
+
+    def forward(self, x):
+        x = self.c_d1(x)
+        x = torch.tanh(x)
+        x = self.c_d1_bn(x)
+        x = self.c_d1_drop(x)
+
+        x = self.c_d2(x)
+        logits = F.log_softmax(x, dim=1)
+
+        return logits
+
+    def loss(self, labels, logits):
+        nll = F.nll_loss(logits, labels)
+        return nll
+
+    def prepare_data(self):
+        _ = TrialMNIST(root=self.hparams.data_root, train=True, download=True)