Merge branch 'master' into nitpick/remove-redundant-code

Borda · web-flow · commit b9fff943c7ac · 2025-05-19T15:10:49.000+02:00
diff --git a/.github/workflows/_legacy-checkpoints.yml b/.github/workflows/_legacy-checkpoints.yml
@@ -113,7 +113,7 @@ jobs:
 
       - run: pip install -r requirements/ci.txt
       - name: Upload checkpoints to S3
-        if: secrets.AWS_REGION != ''
+        if: ${{ secrets[AWS_REGION] != '' }}
         working-directory: ${{ env.LEGACY_FOLDER }}
         env:
           AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY }}
diff --git a/.github/workflows/ci-checkpoints.yml.disabled b/.github/workflows/ci-checkpoints.yml.disabled
diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml
@@ -94,6 +94,11 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
+        # adding dome more images as Thunder mainly using python 3.10,
+        # and we need to support integrations as for example LitGPT
+        python_version: ["3.10"]
+        pytorch_version: ["2.3.1", "2.4.1", "2.5.1", "2.6.0", "2.7.0"]
+        cuda_version: ["12.4.1"]
         include:
           # These are the base images for PL release docker images.
           # Make sure the matrix here matches the one above.
diff --git a/.github/workflows/release-pkg.yml b/.github/workflows/release-pkg.yml
@@ -13,7 +13,7 @@ on:
       - "requirements/ci.txt"
       - ".github/actions/pkg-check/*"
       - ".github/actions/pkg-publish/*"
-      - ".github/workflows/_legacy-checkpoints.yml.yml"
+      - ".github/workflows/_legacy-checkpoints.yml"
       - ".github/workflows/_build-packages.yml"
       - ".github/workflows/release-pypi.yml"
 
@@ -179,12 +179,12 @@ jobs:
         with:
           pkg-folder: dist/${{ steps.folder.outputs.pkg }}
           pypi-token: ${{ secrets[format('PYPI_TOKEN_{0}', matrix.name)] }}
-
-  legacy-checkpoints:
-    needs: [build-packages]
-    uses: ./.github/workflows/_legacy-checkpoints.yml
-    with:
-      push_to_s3: ${{ startsWith(github.event.ref, 'refs/tags') || github.event_name == 'release' }}
-      upload_local: ${{ startsWith(github.event.ref, 'refs/tags') || github.event_name == 'release' }}
-      create_pr: ${{ startsWith(github.event.ref, 'refs/tags') || github.event_name == 'release' }}
-    secrets: inherit
+# FIXME: this is not working suddenly, Unrecognized named-value: 'secrets'
+#  legacy-checkpoints:
+#    needs: [build-packages]
+#    uses: ./.github/workflows/_legacy-checkpoints.yml
+#    with:
+#      push_to_s3: ${{ startsWith(github.event.ref, 'refs/tags') || github.event_name == 'release' }}
+#      upload_local: ${{ startsWith(github.event.ref, 'refs/tags') || github.event_name == 'release' }}
+#      create_pr: ${{ startsWith(github.event.ref, 'refs/tags') || github.event_name == 'release' }}
+#    secrets: inherit
diff --git a/requirements/ci.txt b/requirements/ci.txt
@@ -1,4 +1,4 @@
-setuptools <80.4.1
+setuptools <80.7.2
 wheel <0.46.0
 awscli >=1.30.0, <1.41.0
 twine ==6.1.0
diff --git a/requirements/docs.txt b/requirements/docs.txt
@@ -1,7 +1,7 @@
 sphinx >5.0, <6.0
-myst-parser >=0.18.1, <3.0.0
+myst-parser >=0.18.1, <4.0.0
 nbsphinx >=0.8.5, <=0.9.7
-nbconvert <7.14  # temporary fix for https://github.com/jupyter/nbconvert/issues/2092
+nbconvert >7.14, <7.17
 pandoc >=1.0, <=2.3
 docutils >=0.16, <0.22
 sphinxcontrib-fulltoc >=1.0, <=1.2.0
@@ -12,7 +12,7 @@ sphinx-paramlinks >=0.5.1, <=0.6.0
 sphinx-togglebutton >=0.2, <=0.3.2
 sphinx-copybutton >=0.3, <=0.5.2
 sphinx-multiproject
-sphinx-toolbox ==3.10.0
+sphinx-toolbox ==4.0.0
 sphinx-rtd-dark-mode
 sphinxcontrib-video ==0.4.1
 jinja2 <3.2.0
diff --git a/requirements/fabric/test.txt b/requirements/fabric/test.txt
@@ -2,8 +2,8 @@ coverage ==7.8.0
 numpy >=1.17.2, <1.27.0
 pytest ==8.3.5
 pytest-cov ==6.1.1
-pytest-timeout ==2.3.1
-pytest-rerunfailures ==12.0
+pytest-timeout ==2.4.0
+pytest-rerunfailures ==15.1
 pytest-random-order ==1.1.1
 click ==8.1.8
 tensorboardX >=2.2, <2.7.0  # min version is set by torch.onnx missing attribute
diff --git a/requirements/pytorch/extra.txt b/requirements/pytorch/extra.txt
@@ -2,7 +2,7 @@
 #  in case you want to preserve/enforce restrictions on the latest compatible version, add "strict" as an in-line comment
 
 # extended list of package dependencies to reach full functionality
-matplotlib>3.1, <3.9.0
+matplotlib>3.1, <3.10.0
 omegaconf >=2.2.3, <2.4.0
 hydra-core >=1.2.0, <1.4.0
 jsonargparse[signatures] >=4.28.0, <=4.40.0
diff --git a/requirements/pytorch/test.txt b/requirements/pytorch/test.txt
@@ -1,15 +1,15 @@
 coverage ==7.8.0
 pytest ==8.3.5
 pytest-cov ==6.1.1
-pytest-timeout ==2.3.1
-pytest-rerunfailures ==12.0
+pytest-timeout ==2.4.0
+pytest-rerunfailures ==15.1
 pytest-random-order ==1.1.1
 
 # needed in tests
 cloudpickle >=1.3, <3.2.0
 scikit-learn >0.22.1, <1.7.0
 numpy >=1.17.2, <1.27.0
-onnx >=1.12.0, <1.18.0
+onnx >=1.12.0, <1.19.0
 onnxruntime >=1.12.0, <1.21.0
 psutil <7.0.1 # for `DeviceStatsMonitor`
 pandas >1.0, <2.3.0  # needed in benchmarks
diff --git a/src/lightning/pytorch/CHANGELOG.md b/src/lightning/pytorch/CHANGELOG.md
@@ -24,7 +24,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 
--
+- Fixed logger_connector has edge case where step can be a float ([#20692](https://github.com/Lightning-AI/pytorch-lightning/issues/20692))
 
 
 ---
diff --git a/src/lightning/pytorch/cli.py b/src/lightning/pytorch/cli.py
@@ -45,11 +45,18 @@
         Namespace,
         class_from_function,
         register_unresolvable_import_paths,
-        set_config_read_mode,
     )
 
     register_unresolvable_import_paths(torch)  # Required until fix https://github.com/pytorch/pytorch/issues/74483
-    set_config_read_mode(fsspec_enabled=True)
+
+    try:
+        from jsonargparse import set_parsing_settings
+
+        set_parsing_settings(config_read_mode_fsspec_enabled=True)
+    except ImportError:
+        from jsonargparse import set_config_read_mode
+
+        set_config_read_mode(fsspec_enabled=True)
 else:
     locals()["ArgumentParser"] = object
     locals()["Namespace"] = object
diff --git a/src/lightning/pytorch/trainer/connectors/logger_connector/logger_connector.py b/src/lightning/pytorch/trainer/connectors/logger_connector/logger_connector.py
@@ -106,12 +106,13 @@ def log_metrics(self, metrics: _OUT_DICT, step: Optional[int] = None) -> None:
         scalar_metrics = convert_tensors_to_scalars(metrics)
 
         if step is None:
-            step = scalar_metrics.pop("step", None)
-
-        if step is None:
-            # added metrics for convenience
-            scalar_metrics.setdefault("epoch", self.trainer.current_epoch)
-            step = self.trainer.fit_loop.epoch_loop._batches_that_stepped
+            step_metric = scalar_metrics.pop("step", None)
+            if step_metric is not None:
+                step = int(step_metric)
+            else:
+                # added metrics for convenience
+                scalar_metrics.setdefault("epoch", self.trainer.current_epoch)
+                step = self.trainer.fit_loop.epoch_loop._batches_that_stepped
 
         # log actual metrics
         for logger in self.trainer.loggers:
diff --git a/tests/tests_pytorch/trainer/connectors/test_logger_connector.py b/tests/tests_pytorch/trainer/connectors/test_logger_connector.py
@@ -0,0 +1,61 @@
+from unittest.mock import MagicMock, patch
+
+from lightning.pytorch import Trainer
+from lightning.pytorch.loggers import Logger
+from lightning.pytorch.trainer.connectors.logger_connector import _LoggerConnector
+
+
+@patch("lightning.pytorch.trainer.connectors.logger_connector.logger_connector.convert_tensors_to_scalars")
+def test_uses_provided_step(mock_convert):
+    """Test that the LoggerConnector uses explicitly provided step to log metrics."""
+
+    trainer = MagicMock(spec=Trainer)
+    trainer.loggers = [logger := MagicMock(spec=Logger)]
+    connector = _LoggerConnector(trainer)
+    mock_convert.return_value.pop.return_value = step = 42
+
+    connector.log_metrics((metrics := {"some_metric": 123}), step=step)
+
+    assert connector._logged_metrics == metrics
+    mock_convert.assert_called_once_with(metrics)
+    logger.log_metrics.assert_called_once_with(metrics=mock_convert.return_value, step=step)
+    logger.save.assert_called_once_with()
+
+
+@patch("lightning.pytorch.trainer.connectors.logger_connector.logger_connector.convert_tensors_to_scalars")
+def test_uses_step_metric(mock_convert):
+    """Test that the LoggerConnector uses explicitly provided step metric to log metrics."""
+
+    trainer = MagicMock(spec=Trainer)
+    trainer.loggers = [logger := MagicMock(spec=Logger)]
+    connector = _LoggerConnector(trainer)
+    mock_convert.return_value.pop.return_value = step = 42.0
+
+    metrics = {"some_metric": 123}
+    connector.log_metrics(logged_metrics := {**metrics, "step": step})
+
+    assert connector._logged_metrics == logged_metrics
+    mock_convert.assert_called_once_with(logged_metrics)
+    logger.log_metrics.assert_called_once_with(metrics=mock_convert.return_value, step=int(step))
+    logger.save.assert_called_once_with()
+
+
+@patch("lightning.pytorch.trainer.connectors.logger_connector.logger_connector.convert_tensors_to_scalars")
+def test_uses_batches_that_stepped(mock_convert):
+    """Test that the LoggerConnector uses implicitly provided batches_that_stepped to log metrics."""
+
+    trainer = MagicMock(spec=Trainer)
+    trainer.fit_loop = MagicMock()
+    trainer.loggers = [logger := MagicMock(spec=Logger)]
+    connector = _LoggerConnector(trainer)
+    mock_convert.return_value.pop.return_value = None
+
+    connector.log_metrics(metrics := {"some_metric": 123})
+
+    assert connector._logged_metrics == metrics
+    mock_convert.assert_called_once_with(metrics)
+    logger.log_metrics.assert_called_once_with(
+        metrics=mock_convert.return_value, step=trainer.fit_loop.epoch_loop._batches_that_stepped
+    )
+    logger.save.assert_called_once_with()
+    mock_convert.return_value.setdefault.assert_called_once_with("epoch", trainer.current_epoch)

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-setuptools <80.4.1`
	`1`	`+setuptools <80.7.2`
`2`	`2`	`wheel <0.46.0`
`3`	`3`	`awscli >=1.30.0, <1.41.0`
`4`	`4`	`twine ==6.1.0`