Unexpected Behaviour with Model Checkpointing and val_check_interval
🐛 Bug

Currently, if we set val_check_interval in the Trainer flags, model checkpointing happens in the middle of an epoch. We expect it to always happen at the end of an epoch, as explained below.
To Reproduce
When running the code sample below, we are presented with the following output:
Epoch 1:  10%|█████▊                                                    | 500/5005 [00:02<00:22, 200.49it/s, loss=1.331]
INFO:lightning:                                                                                                         
Epoch 00000: val_loss reached 1.39875 (best 1.39875), saving model to checkpoints/_ckpt_epoch_0.ckpt as top 1
Epoch 2:  10%|█████▊                                                    | 500/5005 [00:02<00:22, 202.59it/s, loss=1.279]
INFO:lightning:                                                                                                         
Epoch 00001: val_loss reached 1.34045 (best 1.34045), saving model to checkpoints/_ckpt_epoch_1.ckpt as top 1
Epoch 3:  10%|█████▊                                                    | 500/5005 [00:02<00:21, 207.04it/s, loss=1.275]
INFO:lightning:                                                                                                         
Epoch 00002: val_loss reached 1.32092 (best 1.32092), saving model to checkpoints/_ckpt_epoch_2.ckpt as top 1
Epoch 4:  10%|█████▊                                                    | 500/5005 [00:02<00:26, 170.37it/s, loss=1.281]
INFO:lightning:                                                                                                         
Epoch 00003: val_loss  was not in top 1
Epoch 5:  10%|█████▊                                                    | 500/5005 [00:02<00:21, 206.15it/s, loss=1.285]
INFO:lightning:                                                                                                         
Epoch 00004: val_loss  was not in top 1

Here, we have val_check_interval == 0.1. As one can see, the checkpointing happens at the beginning of an epoch, after the first validation check. The 9 validation chekcs that follow do not trigger model checkpointing. We point to the places from which this behavior emerges in the section "Additional context". Moreover, we use the section "Expected behavior" to explain why model checkpointing should happen at the end of an epoch (at least, as the defualt behavior).
Code sample

The Model is not important here, we simply chose a minimal one. Please focus on the trainer flags.
import torch
from torch.nn import functional as F
from torch.utils.data import DataLoader
from torchvision import datasets, transforms

import pytorch_lightning as pl


class Model(pl.LightningModule):

    def __init__(self):
        super().__init__()
        self.l1 = torch.nn.Linear(28 * 28, 10)

    def forward(self, x):
        return torch.relu(self.l1(x.view(x.size(0), -1)))

    def training_step(self, batch, batch_idx):
        x, y = batch
        y_hat = self(x)
        return {'loss': F.cross_entropy(y_hat, y)}

    def validation_step(self, batch, batch_idx):
        x, y = batch
        y_hat = self(x)
        return {'val_loss': F.cross_entropy(y_hat, y)}

    def validation_epoch_end(self, outputs):
        val_loss_mean = torch.stack([x['val_loss'] for x in outputs]).mean()
        return {'val_loss': val_loss_mean}

    def train_dataloader(self):
        return DataLoader(datasets.MNIST('mnist/', train=True, download=True,
                          transform=transforms.ToTensor()), batch_size=32)

    def val_dataloader(self):
        return DataLoader(datasets.MNIST('mnist/', train=False, download=True,
                          transform=transforms.ToTensor()), batch_size=32)

    def configure_optimizers(self):
        return torch.optim.Adam(self.parameters(), lr=0.02)


if __name__ == '__main__':

    checkpoint_callback = pl.callbacks.ModelCheckpoint('checkpoints/', verbose=True)

    trainer = pl.Trainer(gpus=[0], val_check_interval=0.1,
                         checkpoint_callback=checkpoint_callback)
    model = Model()

    trainer.fit(model)
Expected behavior
Assume the following scenario. Our model has val_check_interval == 0.5. We have a dataset with 100 samples that can have one of 2 balanced labels (50 "0"s, 50 "1"s). After the first epoch, our model has "seen" each label 50 times. Now, assume that model checkpointing is triggered for the second epoch. In the worst case (we shuffle the samples), all 50 train samples that we saw in this epoch (recall that checkpointing occurs mid-epoch, before seeing the remaining 50 samples of this epoch) had the label "0". Namely, we save a model that saw 100 "0"s and 50 "1"s.
Currently, in my research, we study biased sub-datasets and we rely on the fact that our original dataset has balanced labels. Consequently, we create biased and unbiased models whose  performance is compared on each of the labels. The behavior described above does not ensure that we have a balanced model when training on a balanced dataset, hurting our research assumptions and affecting our use-case.
Some may argue that this issue represents an enhancement proposal rather than a bug. However, I believe that this issue should be classified as a "bug", for several reasons. Similar to the paragraph above, the described behavior also affects users who rely on "balanced features" or users trying to estimate "feature importance". Thus, affecting a wider range of researchers. In addition, I have not encountered this behavior in the documentation, and it may be overlooked by users who run their checkpoint callback with verbose == False.
In summary, following the reasoning given above, I expect model checkpointing to always occur at the end of an epoch.
Additional context
I believe that I have located the source of this behavior:

  
    
      pytorch-lightning/pytorch_lightning/trainer/training_loop.py
    
    
        Lines 436 to 453
      in
      3a64260
    
  
  
    

        
          
           # --------------- 
        

        
          
           # RUN VAL STEP 
        

        
          
           # --------------- 
        

        
          
           is_val_check_batch = (batch_idx + 1) % self.val_check_batch == 0 
        

        
          
           can_check_epoch = (self.current_epoch + 1) % self.check_val_every_n_epoch == 0 
        

        
          
           can_check_val = not self.disable_validation and can_check_epoch 
        

        
          
           should_check_val = is_val_check_batch or early_stop_epoch 
        

        
          
           should_check_val = should_check_val or (is_last_batch and self.val_check_batch == float('inf')) 
        

        
          
           should_check_val = can_check_val and should_check_val 
        

        
          
            
        

        
          
           # --------------- 
        

        
          
           # CHECKPOINTING, EARLY STOPPING 
        

        
          
           # --------------- 
        

        
          
           # fast_dev_run always forces val checking after train batch 
        

        
          
           if self.fast_dev_run or should_check_val: 
        

        
          
               self.run_evaluation(test_mode=self.testing) 
        

        
          
               self.call_checkpoint_callback() 
        

        
          
               self.call_early_stop_callback() 
        
    
  


For example, if val_check_interval == 0.1, the variable should_check_val is True 10 times during an epoch and self.call_checkpoint_callback() is called.
Then, the following if-statement fails at the first validation run of each epoch and saves the checkpoint. Afterwards, the 9 validation runs that follow, enter this if-statement and skip saving the model.

  
    
      pytorch-lightning/pytorch_lightning/callbacks/model_checkpoint.py
    
    
        Lines 210 to 212
      in
      25bbd05
    
  
  
    

        
          
           if self.epoch_last_check is not None and (epoch - self.epoch_last_check) < self.period: 
        

        
          
               # skipping in this term 
        

        
          
               return 
        
    
  


(As a side note, removing this line will result in a behavior that was requested in #1758 , but obviously, it belongs to a completely different discussion.)

Environment

OS: Linux
PyTorch Version: 1.4.0
How you installed PyTorch: conda
PyTorch Lightning Version: 0.7.5
Python version: 3.6.10