Skip to content

fix: Skip val_check_interval validation when limit_val_batches=0 - #21560

Merged
justusschock merged 13 commits into
Lightning-AI:masterfrom
ManasVardhan:fix/skip-val-check-interval-when-val-disabled
Mar 6, 2026
Merged

fix: Skip val_check_interval validation when limit_val_batches=0#21560
justusschock merged 13 commits into
Lightning-AI:masterfrom
ManasVardhan:fix/skip-val-check-interval-when-val-disabled

Conversation

@ManasVardhan

@ManasVardhan ManasVardhan commented Feb 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #21553

When limit_val_batches=0 (validation disabled), the val_check_interval sanity check is now skipped since validation won't run anyway.

Changes

  • In _FitLoop.setup_data(), added an early check: when trainer.limit_val_batches == 0, set val_check_batch = inf and skip all interval validation logic.
  • Added regression test confirming val_check_interval > limit_train_batches no longer raises when validation is disabled.

📚 Documentation preview 📚: https://pytorch-lightning--21560.org.readthedocs.build/en/21560/

When limit_val_batches=0 (validation disabled), the val_check_interval
sanity check is now skipped since validation will not run anyway.

Fixes #21553
@github-actions github-actions Bot added the pl Generic label for PyTorch Lightning package label Feb 28, 2026
Comment thread src/lightning/pytorch/loops/fit_loop.py Outdated
Comment thread tests/tests_pytorch/loops/test_training_loop.py Outdated
@codecov

codecov Bot commented Feb 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79%. Comparing base (8e805f9) to head (fc89bb4).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

❗ There is a different number of reports uploaded between BASE (8e805f9) and HEAD (fc89bb4). Click for more details.

HEAD has 726 uploads less than BASE
Flag BASE (8e805f9) HEAD (fc89bb4)
cpu 198 33
python 18 3
lightning_fabric 54 0
pytest 99 0
python3.10 18 3
lightning 90 15
python3.12 54 9
python3.12.7 54 9
python3.13 18 3
python3.11 36 6
pytorch2.3 9 3
pytest-full 99 33
pytorch2.1 18 6
pytorch2.8 18 6
pytorch_lightning 54 18
pytorch2.5.1 9 3
pytorch2.7 9 3
pytorch2.2.2 9 3
pytorch2.4.1 9 3
pytorch2.9 9 3
pytorch2.6 9 3
Additional details and impacted files
@@            Coverage Diff            @@
##           master   #21560     +/-   ##
=========================================
- Coverage      87%      79%     -8%     
=========================================
  Files         270      267      -3     
  Lines       24077    24018     -59     
=========================================
- Hits        20865    18961   -1904     
- Misses       3212     5057   +1845     

@ManasVardhan

Copy link
Copy Markdown
Contributor Author

Addressed both review comments:

  1. Moved comments inside the if-elif blocks
  2. Removed the extra blank line in the test docstring

Thanks for the review @deependujha!

@deependujha

Copy link
Copy Markdown
Collaborator

thanks. Can you update the changelog too.

Comment thread tests/tests_pytorch/loops/test_training_loop.py Outdated
@ManasVardhan

Copy link
Copy Markdown
Contributor Author

Updated:

  • Added changelog entry under [unreleased] > Fixed
  • Collapsed test docstring to single line

Thanks for the review!

Comment thread src/lightning/pytorch/loops/fit_loop.py Outdated
@taha-yassine

Copy link
Copy Markdown
Contributor

Hey @ManasVardhan, thanks for picking this up so quickly. I was planning to submit a PR as well. Since I opened the original issue and proposed the solution, would you be open to adding me as a co-author?

@ManasVardhan

Copy link
Copy Markdown
Contributor Author

Hey @ManasVardhan, thanks for picking this up so quickly. I was planning to submit a PR as well. Since I opened the original issue and proposed the solution, would you be open to adding me as a co-author?

Hi Taha, yes absolutely.

@ManasVardhan

Copy link
Copy Markdown
Contributor Author

Re: why not None instead of float("inf"):

val_check_batch is compared against float("inf") in training_epoch_loop.py (lines 424 and 438) to determine validation scheduling. Using None would break those comparisons. float("inf") ensures the validation check condition is never satisfied, which is the correct behavior when validation is disabled.

Co-authored-by: Taha Yassine <40228615+taha-yassine@users.noreply.github.com>
@deependujha

Copy link
Copy Markdown
Collaborator

not sure with your reasoning.

@ManasVardhan

ManasVardhan commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

@deependujha Re: why float("inf") instead of None - val_check_batch is compared against float("inf") downstream in training_epoch_loop.py (line 535: is_infinite_dataset = self.trainer.val_check_batch == float("inf")), and when it's not inf, there's an assert self.trainer.val_check_batch is not None on line 556 before using it in a modulo operation. Setting it to None would risk hitting that assertion. float("inf") follows the existing convention for "no batch-based validation check" (same value used for iterable datasets without a defined length).

@deependujha

Copy link
Copy Markdown
Collaborator

thanks for the clarification, but i believe, the line won't even get executed.

def _should_check_val_epoch(self) -> bool:
return self.trainer.enable_validation and (
self.trainer.check_val_every_n_epoch is None
or (self.trainer.current_epoch + 1) % self.trainer.check_val_every_n_epoch == 0
)
def _should_check_val_fx(self, data_fetcher: _DataFetcher) -> bool:
"""Decide if we should run validation."""
if not self._should_check_val_epoch():
return False
# val_check_batch is inf for iterable datasets with no length defined
is_infinite_dataset = self.trainer.val_check_batch == float("inf")
is_last_batch = self.batch_progress.is_last_batch

_should_check_val_fx calls _should_check_val_epoch, and it calls self.trainer.enable_validation, which will return False, and hence, the function should_check_val_fx will return False (line 532), and those inf checks & assertion won't even run.

@property
def enable_validation(self) -> bool:
"""Check if we should run validation during training."""
return (
self.fit_loop.epoch_loop.val_loop._data_source.is_defined()
and is_overridden("validation_step", self.lightning_module)
and self.limit_val_batches > 0
)

  • limit_val_batches is already set to 0, so will return False.

@ManasVardhan

ManasVardhan commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Good catch @deependujha! You're right, when limit_val_batches=0, enable_validation returns False so none of those inf checks or assertions ever run. None would totally work here too.

I went with float("inf") just because that's what the rest of the codebase uses for "skip this", but honestly it doesn't matter much in this case. Happy to switch to None if you think that reads better - lmk!

@deependujha

deependujha commented Mar 3, 2026

Copy link
Copy Markdown
Collaborator

thanks, but I believe the current changes are enough. Thanks to both of you for the great work. Should be landed soon.

@deependujha deependujha added the ready to be merged PRs ready to be merged label Mar 4, 2026
@ManasVardhan

Copy link
Copy Markdown
Contributor Author

CI is all green -- ready to merge whenever you get a chance! Thanks for the review 🙏

@justusschock
justusschock merged commit 9dc525c into Lightning-AI:master Mar 6, 2026
91 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pl Generic label for PyTorch Lightning package ready to be merged PRs ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

val_check_interval check not bypassed when limit_val_batches=0

4 participants