From ecc6a9bc4a1cd9b67cad810b864cbd113142020c Mon Sep 17 00:00:00 2001 From: GdoongMathew Date: Tue, 7 Oct 2025 00:07:30 +0800 Subject: [PATCH 1/3] refactor: try catch a more specific exception during log_metrics. --- src/lightning/fabric/loggers/tensorboard.py | 3 +-- tests/tests_pytorch/loggers/test_tensorboard.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lightning/fabric/loggers/tensorboard.py b/src/lightning/fabric/loggers/tensorboard.py index 208244dc38cd3..b33377f8a9434 100644 --- a/src/lightning/fabric/loggers/tensorboard.py +++ b/src/lightning/fabric/loggers/tensorboard.py @@ -211,8 +211,7 @@ def log_metrics(self, metrics: Mapping[str, float], step: Optional[int] = None) else: try: self.experiment.add_scalar(k, v, step) - # TODO(fabric): specify the possible exception - except Exception as ex: + except (NotImplementedError, ValueError) as ex: raise ValueError( f"\n you tried to log {v} which is currently not supported. Try a dict or a scalar/tensor." ) from ex diff --git a/tests/tests_pytorch/loggers/test_tensorboard.py b/tests/tests_pytorch/loggers/test_tensorboard.py index 7e02a73c93082..751224b93d3a6 100644 --- a/tests/tests_pytorch/loggers/test_tensorboard.py +++ b/tests/tests_pytorch/loggers/test_tensorboard.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import os +import re from argparse import Namespace from unittest import mock from unittest.mock import Mock @@ -157,6 +158,18 @@ def test_tensorboard_log_metrics(tmp_path, step_idx): logger.log_metrics(metrics, step_idx) +@pytest.mark.parametrize("value", [[1], "x", None]) +def test_tensorboard_log_metrics_exception_message(tmp_path, value): + logger = TensorBoardLogger(tmp_path) + with pytest.raises( + ValueError, + match=re.escape( + f"you tried to log {value} which is currently not supported. Try a dict or a scalar/tensor.", + ), + ): + logger.log_metrics(metrics={"metric": value}) + + def test_tensorboard_log_hyperparams(tmp_path): logger = TensorBoardLogger(tmp_path) hparams = { From e7d5cb2d84d91a700bc7a7b9a853b784303404be Mon Sep 17 00:00:00 2001 From: GdoongMathew Date: Wed, 8 Oct 2025 02:33:38 +0800 Subject: [PATCH 2/3] fix test. --- src/lightning/fabric/loggers/tensorboard.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/lightning/fabric/loggers/tensorboard.py b/src/lightning/fabric/loggers/tensorboard.py index b33377f8a9434..010e8b55636c3 100644 --- a/src/lightning/fabric/loggers/tensorboard.py +++ b/src/lightning/fabric/loggers/tensorboard.py @@ -211,7 +211,11 @@ def log_metrics(self, metrics: Mapping[str, float], step: Optional[int] = None) else: try: self.experiment.add_scalar(k, v, step) - except (NotImplementedError, ValueError) as ex: + except ( + NotImplementedError, + ValueError, + ModuleNotFoundError, # https://github.com/pytorch/pytorch/issues/24175 + ) as ex: raise ValueError( f"\n you tried to log {v} which is currently not supported. Try a dict or a scalar/tensor." ) from ex From e5c3f14065c4ec934c70c6734ebe7a20d20090eb Mon Sep 17 00:00:00 2001 From: GdoongMathew Date: Wed, 8 Oct 2025 17:46:04 +0800 Subject: [PATCH 3/3] Apply suggestion from @GdoongMathew --- src/lightning/fabric/loggers/tensorboard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lightning/fabric/loggers/tensorboard.py b/src/lightning/fabric/loggers/tensorboard.py index 010e8b55636c3..94c3642a8cb64 100644 --- a/src/lightning/fabric/loggers/tensorboard.py +++ b/src/lightning/fabric/loggers/tensorboard.py @@ -214,7 +214,7 @@ def log_metrics(self, metrics: Mapping[str, float], step: Optional[int] = None) except ( NotImplementedError, ValueError, - ModuleNotFoundError, # https://github.com/pytorch/pytorch/issues/24175 + ModuleNotFoundError, # this exception only applies to torch < 2.4 (since caffe2 was dropped later) https://github.com/pytorch/pytorch/issues/24175 ) as ex: raise ValueError( f"\n you tried to log {v} which is currently not supported. Try a dict or a scalar/tensor."