Merge branch 'feature/13324_validation-interval' of https://github.com/Sohaib-Ahmed21/pytorch-lightning into feature/13324_validation-interval

Author: Sohaib-Ahmed21

src/lightning/pytorch/callbacks/model_checkpoint.py

@@ -472,7 +472,7 @@ def _should_save_on_train_epoch_end(self, trainer: "pl.Trainer") -> bool:
         # time-based validation: always defer saving to validation end
         if getattr(trainer, "_val_check_time_interval", None) is not None:
             return False
-        
+
         # if `check_val_every_n_epoch != 1`, we can't say when the validation dataloader will be loaded
         # so let's not enforce saving at every training epoch end
         if trainer.check_val_every_n_epoch != 1:

src/lightning/pytorch/loops/evaluation_loop.py

@@ -15,11 +15,11 @@
 import os
 import shutil
 import sys
+import time
 from collections import ChainMap, OrderedDict, defaultdict
 from collections.abc import Iterable, Iterator
 from dataclasses import dataclass
 from typing import Any, Optional, Union
-import time
 
 from lightning_utilities.core.apply_func import apply_to_collection
 from torch import Tensor
@@ -314,7 +314,7 @@ def on_run_end(self) -> list[_OUT_DICT]:
 
         if self.verbose and self.trainer.is_global_zero:
             self._print_results(logged_outputs, self._stage.value)
-        
+
         now = time.monotonic()
         self.trainer._last_val_time = now
 

src/lightning/pytorch/loops/fit_loop.py

@@ -12,9 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
+import time
 from dataclasses import dataclass
 from typing import Any, Optional, Union
-import time
 
 import torch
 from typing_extensions import override
@@ -289,7 +289,7 @@ def setup_data(self) -> None:
         if getattr(trainer, "_val_check_time_interval", None) is not None:
             trainer.val_check_batch = None
             trainer._train_start_time = time.monotonic()
-            trainer._last_val_time   = trainer._train_start_time
+            trainer._last_val_time = trainer._train_start_time
         elif isinstance(trainer.val_check_interval, int):
             trainer.val_check_batch = trainer.val_check_interval
             if trainer.val_check_batch > self.max_batches and trainer.check_val_every_n_epoch is not None:

src/lightning/pytorch/loops/training_epoch_loop.py

@@ -13,10 +13,10 @@
 # limitations under the License.
 import contextlib
 import math
+import time
 from collections import OrderedDict
 from dataclasses import dataclass
 from typing import Any, Optional, Union
-import time
 
 import torch
 from typing_extensions import override

src/lightning/pytorch/trainer/connectors/data_connector.py

@@ -14,8 +14,8 @@
 import os
 from collections.abc import Iterable
 from dataclasses import dataclass, field
-from typing import Any, Optional, Union
 from datetime import timedelta
+from typing import Any, Optional, Union
 
 import torch.multiprocessing as mp
 from torch.utils.data import BatchSampler, DataLoader, RandomSampler, Sampler, SequentialSampler

src/lightning/pytorch/trainer/setup.py

@@ -13,8 +13,8 @@
 # limitations under the License.
 """Houses the methods used to set up the Trainer."""
 
-from typing import Optional, Union
 from datetime import timedelta
+from typing import Optional, Union
 
 import lightning.pytorch as pl
 from lightning.fabric.utilities.warnings import PossibleUserWarning
@@ -199,29 +199,30 @@ def _log_device_info(trainer: "pl.Trainer") -> None:
         if HPUAccelerator.is_available() and not isinstance(trainer.accelerator, HPUAccelerator):
             rank_zero_warn("HPU available but not used. You can set it by doing `Trainer(accelerator='hpu')`.")
 
+
 def _parse_time_interval_seconds(value: Union[str, timedelta, dict]) -> float:
-     if isinstance(value, timedelta):
-         return value.total_seconds()
-     if isinstance(value, dict):
-         td = timedelta(**value)
-         return td.total_seconds()
-     if isinstance(value, str):
-         parts = value.split(":")
-         if len(parts) != 4:
-             raise MisconfigurationException(
-                 f"Invalid time format for `val_check_interval`: {value!r}. Expected 'DD:HH:MM:SS'."
-             )
-         d, h, m, s = parts
-         try:
-             days = int(d)
-             hours = int(h)
-             minutes = int(m)
-             seconds = int(s)
-         except ValueError:
-             raise MisconfigurationException(
-                 f"Non-integer component in `val_check_interval` string: {value!r}. Use 'DD:HH:MM:SS'."
-             )
-         td = timedelta(days=days, hours=hours, minutes=minutes, seconds=seconds)
-         return td.total_seconds()
-     # Should not happen given the caller guards
-     raise MisconfigurationException(f"Unsupported type for `val_check_interval`: {type(value)!r}")
+    if isinstance(value, timedelta):
+        return value.total_seconds()
+    if isinstance(value, dict):
+        td = timedelta(**value)
+        return td.total_seconds()
+    if isinstance(value, str):
+        parts = value.split(":")
+        if len(parts) != 4:
+            raise MisconfigurationException(
+                f"Invalid time format for `val_check_interval`: {value!r}. Expected 'DD:HH:MM:SS'."
+            )
+        d, h, m, s = parts
+        try:
+            days = int(d)
+            hours = int(h)
+            minutes = int(m)
+            seconds = int(s)
+        except ValueError:
+            raise MisconfigurationException(
+                f"Non-integer component in `val_check_interval` string: {value!r}. Use 'DD:HH:MM:SS'."
+            )
+        td = timedelta(days=days, hours=hours, minutes=minutes, seconds=seconds)
+        return td.total_seconds()
+    # Should not happen given the caller guards
+    raise MisconfigurationException(f"Unsupported type for `val_check_interval`: {type(value)!r}")

src/lightning/pytorch/trainer/trainer.py

@@ -109,7 +109,7 @@ def __init__(
         limit_test_batches: Optional[Union[int, float]] = None,
         limit_predict_batches: Optional[Union[int, float]] = None,
         overfit_batches: Union[int, float] = 0.0,
-        val_check_interval: Optional[Union[int, float, str, timedelta, dict[str,int]]] = None,
+        val_check_interval: Optional[Union[int, float, str, timedelta, dict[str, int]]] = None,
         check_val_every_n_epoch: Optional[int] = 1,
         num_sanity_val_steps: Optional[int] = None,
         log_every_n_steps: Optional[int] = None,
@@ -212,7 +212,7 @@ def __init__(
 
             check_val_every_n_epoch: Perform a validation loop after every `N` training epochs. If ``None``,
                 validation will be done solely based on the number of training batches, requiring ``val_check_interval``
-                to be an integer value. When used together with a time-based ``val_check_interval`` and 
+                to be an integer value. When used together with a time-based ``val_check_interval`` and
                 ``check_val_every_n_epoch`` > 1, validation is aligned to epoch multiples: if the interval elapses
                 before the next multiple-N epoch, validation runs at the start of that epoch (after the first batch)
                 and the timer resets; if it elapses during a multiple-N epoch, validation runs after the current batch.

tests/tests_pytorch/trainer/flags/test_val_check_interval.py

@@ -12,11 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
-
-import pytest
-import time
 import re
+import time
 from unittest.mock import patch
+
+import pytest
 from torch.utils.data import DataLoader
 
 from lightning.pytorch.demos.boring_classes import BoringModel, RandomDataset, RandomIterableDataset
@@ -132,17 +132,19 @@ def test_val_check_interval_float_with_none_check_val_every_n_epoch():
     with pytest.raises(
         MisconfigurationException,
         match=re.escape(
-        "`val_check_interval` should be an integer or a time-based duration (str 'DD:HH:MM:SS', "
-        "datetime.timedelta, or dict kwargs for timedelta) when `check_val_every_n_epoch=None`."
-    )
+            "`val_check_interval` should be an integer or a time-based duration (str 'DD:HH:MM:SS', "
+            "datetime.timedelta, or dict kwargs for timedelta) when `check_val_every_n_epoch=None`."
+        ),
     ):
         Trainer(
             val_check_interval=0.5,
             check_val_every_n_epoch=None,
         )
 
+
 def test_time_based_val_check_interval(tmp_path):
     call_count = {"count": 0}
+
     def fake_time():
         result = call_count["count"]
         call_count["count"] += 2
@@ -168,17 +170,20 @@ def fake_time():
 
 
 @pytest.mark.parametrize(
-    "check_val_every_n_epoch, val_check_interval, epoch_duration, expected_val_batches, description",
+    ("check_val_every_n_epoch", "val_check_interval", "epoch_duration", "expected_val_batches", "description"),
     [
         (None, "00:00:00:04", 2, [0, 1, 0, 1, 0], "val_check_interval timer only, no epoch gating"),
         (1, "00:00:00:06", 8, [1, 1, 2, 1, 1], "val_check_interval timer only, no epoch gating"),
         (2, "00:00:00:06", 9, [0, 2, 0, 2, 0], "epoch gating, timer longer than epoch"),
         (2, "00:00:00:20", 9, [0, 0, 0, 1, 0], "epoch gating, timer much longer"),
         (2, "00:00:00:03", 9, [0, 3, 0, 3, 0], "epoch gating, timer shorter than epoch"),
-    ]
+    ],
 )
-def test_time_and_epoch_gated_val_check(tmp_path, check_val_every_n_epoch, val_check_interval, epoch_duration, expected_val_batches, description):
+def test_time_and_epoch_gated_val_check(
+    tmp_path, check_val_every_n_epoch, val_check_interval, epoch_duration, expected_val_batches, description
+):
     call_count = {"count": 0}
+
     # Simulate time in steps (each batch is 1 second, epoch_duration=seconds per epoch)
     def fake_time():
         result = call_count["count"]
@@ -191,7 +196,11 @@ class TestModel(BoringModel):
         val_epoch_calls = 0
 
         def on_train_batch_end(self, *args, **kwargs):
-            if isinstance(self.trainer.check_val_every_n_epoch,int) and self.trainer.check_val_every_n_epoch > 1 and (self.trainer.current_epoch + 1) % self.trainer.check_val_every_n_epoch != 0:
+            if (
+                isinstance(self.trainer.check_val_every_n_epoch, int)
+                and self.trainer.check_val_every_n_epoch > 1
+                and (self.trainer.current_epoch + 1) % self.trainer.check_val_every_n_epoch != 0
+            ):
                 time.monotonic()
 
         def on_train_epoch_end(self, *args, **kwargs):
@@ -205,17 +214,17 @@ def on_validation_epoch_start(self) -> None:
     max_steps = max_epochs * epoch_duration
     limit_train_batches = epoch_duration
 
-    trainer_kwargs = dict(
-        default_root_dir=tmp_path,
-        logger=False,
-        enable_checkpointing=False,
-        max_epochs=max_epochs,
-        max_steps=max_steps,
-        limit_val_batches=1,
-        limit_train_batches=limit_train_batches,
-        val_check_interval=val_check_interval,
-        check_val_every_n_epoch=check_val_every_n_epoch
-    )
+    trainer_kwargs = {
+        "default_root_dir": tmp_path,
+        "logger": False,
+        "enable_checkpointing": False,
+        "max_epochs": max_epochs,
+        "max_steps": max_steps,
+        "limit_val_batches": 1,
+        "limit_train_batches": limit_train_batches,
+        "val_check_interval": val_check_interval,
+        "check_val_every_n_epoch": check_val_every_n_epoch,
+    }
 
     with patch("time.monotonic", side_effect=fake_time):
         model = TestModel()
@@ -227,4 +236,4 @@ def on_validation_epoch_start(self) -> None:
         f"\nFAILED: {description}"
         f"\nExpected validation at batches: {expected_val_batches},"
         f"\nGot: {model.val_batches, model.val_epoch_calls}\n"
-    )
+    )