Tuner starts from validation even though num_sanity_val_steps is set to 0

[fierval]

Bug description

In PL 1.9.0:

tuner = pl.Trainer(auto_lr_find=True, accelerator="cuda", num_sanity_val_steps=0)
tuner.tune(model_module)

The model_module is Lightning module that has a field called lr.

Starts validation even though num_sanity_val_steps=0.

Calling fit() works fine
Also works fine in 1.7.7

How to reproduce the bug

No response

Error messages and logs

# Error messages and logs here please

Environment

Current environment

#- Lightning Component (e.g. Trainer, LightningModule, LightningApp, LightningWork, LightningFlow):
#- PyTorch Lightning Version (e.g., 1.5.0):
#- Lightning App Version (e.g., 0.5.2):
#- PyTorch Version (e.g., 1.10):
#- Python version (e.g., 3.9):
#- OS (e.g., Linux):
#- CUDA/cuDNN version:
#- GPU models and configuration:
#- How you installed Lightning(`conda`, `pip`, source):
#- Running environment of LightningApp (e.g. local, cloud):

More info

No response

cc @Borda

[carmocca]

Repro:

import os

import torch
from torch.utils.data import DataLoader, Dataset

from pytorch_lightning import LightningModule, Trainer


class RandomDataset(Dataset):
    def __init__(self, size, length):
        self.len = length
        self.data = torch.randn(length, size)

    def __getitem__(self, index):
        return self.data[index]

    def __len__(self):
        return self.len


class BoringModel(LightningModule):
    def __init__(self, lr: float = 0.1):
        super().__init__()
        self.lr = lr
        self.layer = torch.nn.Linear(32, 2)

    def forward(self, x):
        return self.layer(x)

    def training_step(self, batch, batch_idx):
        print("run training!", batch_idx)
        return self(batch).sum()

    def validation_step(self, batch, batch_idx):
        if self.trainer.sanity_checking:
            raise ValueError
        print("run validation!", batch_idx)

    def configure_optimizers(self):
        return torch.optim.SGD(self.layer.parameters(), lr=self.lr)


def run():
    train_data = DataLoader(RandomDataset(32, 64), batch_size=2)
    val_data = DataLoader(RandomDataset(32, 64), batch_size=2)

    model = BoringModel()
    trainer = Trainer(
        default_root_dir=os.getcwd(),
        limit_train_batches=1,
        limit_val_batches=1,
        num_sanity_val_steps=0,
        max_epochs=1,
        enable_model_summary=False,
        auto_lr_find=True
    )
    trainer.tune(model, train_dataloaders=train_data, val_dataloaders=val_data)


if __name__ == "__main__":
    run()

[bkiat1123]

I've investigated it, the initial validation is not a sanity check step. In the training_epoch_loop, the initial training step is skipped and validation is run in on_advance_end.

    def advance(self, data_fetcher: AbstractDataFetcher) -> None:
        """Runs a single training batch.

        Raises:
            StopIteration: When the epoch is canceled by the user returning -1
        """
        if self.restarting and self._should_check_val_fx():
            # skip training and run validation in `on_advance_end`
            return

_should_check_val_fx return True is a bit unfortunate.

    is_val_check_batch = (current_iteration + 1) % self.trainer.val_check_batch == 0

Current_iteration is -1, and 0 mod any number is always 0, so it returns True.

The real bug is in the self.restarting part, the loop wasn't able to reset the restarting property to False.

https://github.com/Lightning-AI/lightning/blob/fc195b95405e9e2629466e5b28c6a9243209d596/src/pytorch_lightning/tuner/lr_finder.py#L477-L481

load_state_dict set restarting to True, the code trying to set it back to False. Updating the value of Fit loop restarting property will also update its children loop's restarting property, i.e. the training_epoch_loop

However, a bug in the fit_loop property setter method prevented it.

https://github.com/Lightning-AI/lightning/blob/fc195b95405e9e2629466e5b28c6a9243209d596/src/pytorch_lightning/loops/fit_loop.py#L117-L123

I think the code is trying to convery, is we cannot set restarting to True when both

1. Epoch is finished
2. It is not iteration based training (Otherwise, epoch is irrelevant).

There isn't any restriction on setting restarting to False.

It should be written as

restarting = restarting and (epoch_unfinished or self._iteration_based_training())

• When we set restarting to False, it always return False.
• When we set restarting to True, either epoch_unfinished or _iteration_based_training has to be True also.

What happened in the tuning bug is

restarting = False
epoch_unfinished = True
self._iteration_based_training() = True

False and True or True == True
False and (True or True) == False

So restarting remain True, self._should_check_val_fx() also return True, the training is skipped, and validation is run.

@awaelchli , happy to open a PR if you agreed on the diagnosis and solution.