Merge branch 'master' into feature/option-disable-loghparams

Author: duydl

src/lightning/pytorch/CHANGELOG.md

@@ -16,6 +16,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 
+- Fix CSVLogger logging hyperparameter at every write which increase latency  ([#20594](https://github.com/Lightning-AI/pytorch-lightning/pull/20594))
+
+
 ## [2.5.0] - 2024-12-19
 
 ### Added

src/lightning/pytorch/loggers/csv_logs.py

@@ -55,15 +55,10 @@ def __init__(self, log_dir: str) -> None:
         self.hparams: dict[str, Any] = {}
 
     def log_hparams(self, params: dict[str, Any]) -> None:
-        """Record hparams."""
+        """Record hparams and save into files."""
         self.hparams.update(params)
-
-    @override
-    def save(self) -> None:
-        """Save recorded hparams and metrics into files."""
         hparams_file = os.path.join(self.log_dir, self.NAME_HPARAMS_FILE)
         save_hparams_to_yaml(hparams_file, self.hparams)
-        return super().save()
 
 
 class CSVLogger(Logger, FabricCSVLogger):
@@ -144,7 +139,7 @@ def save_dir(self) -> str:
 
     @override
     @rank_zero_only
-    def log_hyperparams(self, params: Union[dict[str, Any], Namespace]) -> None:
+    def log_hyperparams(self, params: Optional[Union[dict[str, Any], Namespace]] = None) -> None:
         params = _convert_params(params)
         self.experiment.log_hparams(params)
 

tests/tests_fabric/utilities/test_seed.py

@@ -1,5 +1,6 @@
 import os
 import random
+import warnings
 from unittest import mock
 from unittest.mock import Mock
 
@@ -30,9 +31,9 @@ def test_seed_stays_same_with_multiple_seed_everything_calls():
         seed_everything()
     initial_seed = os.environ.get("PL_GLOBAL_SEED")
 
-    with pytest.warns(None) as record:
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
         seed_everything()
-    assert not record  # does not warn
     seed = os.environ.get("PL_GLOBAL_SEED")
 
     assert initial_seed == seed

tests/tests_pytorch/callbacks/test_lr_monitor.py

@@ -548,10 +548,10 @@ def finetune_function(self, pl_module, epoch: int, optimizer):
             """Called when the epoch begins."""
             if epoch == 1 and isinstance(optimizer, torch.optim.SGD):
                 self.unfreeze_and_add_param_group(pl_module.backbone[0], optimizer, lr=0.1)
-            if epoch == 2 and isinstance(optimizer, torch.optim.Adam):
+            if epoch == 2 and type(optimizer) is torch.optim.Adam:
                 self.unfreeze_and_add_param_group(pl_module.layer, optimizer, lr=0.1)
 
-            if epoch == 3 and isinstance(optimizer, torch.optim.Adam):
+            if epoch == 3 and type(optimizer) is torch.optim.Adam:
                 assert len(optimizer.param_groups) == 2
                 self.unfreeze_and_add_param_group(pl_module.backbone[1], optimizer, lr=0.1)
                 assert len(optimizer.param_groups) == 3

tests/tests_pytorch/loggers/test_csv.py

@@ -75,7 +75,6 @@ def test_named_version(tmp_path):
 
     logger = CSVLogger(save_dir=tmp_path, name=exp_name, version=expected_version)
     logger.log_hyperparams({"a": 1, "b": 2})
-    logger.save()
     assert logger.version == expected_version
     assert os.listdir(tmp_path / exp_name) == [expected_version]
     assert os.listdir(tmp_path / exp_name / expected_version)
@@ -85,7 +84,7 @@ def test_named_version(tmp_path):
 def test_no_name(tmp_path, name):
     """Verify that None or empty name works."""
     logger = CSVLogger(save_dir=tmp_path, name=name)
-    logger.save()
+    logger.log_hyperparams()
     assert os.path.normpath(logger.root_dir) == str(tmp_path)  # use os.path.normpath to handle trailing /
     assert os.listdir(tmp_path / "version_0")
 
@@ -116,7 +115,6 @@ def test_log_hyperparams(tmp_path):
         "layer": torch.nn.BatchNorm1d,
     }
     logger.log_hyperparams(hparams)
-    logger.save()
 
     path_yaml = os.path.join(logger.log_dir, ExperimentWriter.NAME_HPARAMS_FILE)
     params = load_hparams_from_yaml(path_yaml)