From 029c7ab4752cf7c4e76bf1a6077404a36fb06a4c Mon Sep 17 00:00:00 2001 From: Bastien Enjalbert Date: Tue, 11 Mar 2025 15:18:13 +0100 Subject: [PATCH 1/6] feat[logger] update mlflow limit for parameters length log using mlflow package --- src/lightning/pytorch/loggers/mlflow.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/lightning/pytorch/loggers/mlflow.py b/src/lightning/pytorch/loggers/mlflow.py index 1d158f41b52bc..7e8125bcc4dab 100644 --- a/src/lightning/pytorch/loggers/mlflow.py +++ b/src/lightning/pytorch/loggers/mlflow.py @@ -235,10 +235,17 @@ def log_hyperparams(self, params: Union[dict[str, Any], Namespace]) -> None: params = _flatten_dict(params) from mlflow.entities import Param - - # Truncate parameter values to 250 characters. - # TODO: MLflow 1.28 allows up to 500 characters: https://github.com/mlflow/mlflow/releases/tag/v1.28.0 - params_list = [Param(key=k, value=str(v)[:250]) for k, v in params.items()] + + import mlflow.utils.validation + + # Check maximum param value length is available and use it + if hasattr(mlflow.utils.validation, 'MAX_PARAM_VAL_LENGTH'): + param_length_limit = mlflow.utils.validation.MAX_PARAM_VAL_LENGTH + else: # Fallback + param_length_limit = 250 # Historical default value + + # Use mlflow default limit or truncate parameter values to 250 characters if limit is not available + params_list = [Param(key=k, value=str(v)[:param_length_limit]) for k, v in params.items()] # Log in chunks of 100 parameters (the maximum allowed by MLflow). for idx in range(0, len(params_list), 100): From ef75b6f5e99d6efdf377795602a2f623c28a327c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 11 Mar 2025 14:24:05 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/lightning/pytorch/loggers/mlflow.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lightning/pytorch/loggers/mlflow.py b/src/lightning/pytorch/loggers/mlflow.py index 7e8125bcc4dab..5a2f5ccd6dc89 100644 --- a/src/lightning/pytorch/loggers/mlflow.py +++ b/src/lightning/pytorch/loggers/mlflow.py @@ -234,14 +234,13 @@ def log_hyperparams(self, params: Union[dict[str, Any], Namespace]) -> None: params = _convert_params(params) params = _flatten_dict(params) - from mlflow.entities import Param - import mlflow.utils.validation - + from mlflow.entities import Param + # Check maximum param value length is available and use it - if hasattr(mlflow.utils.validation, 'MAX_PARAM_VAL_LENGTH'): + if hasattr(mlflow.utils.validation, "MAX_PARAM_VAL_LENGTH"): param_length_limit = mlflow.utils.validation.MAX_PARAM_VAL_LENGTH - else: # Fallback + else: # Fallback param_length_limit = 250 # Historical default value # Use mlflow default limit or truncate parameter values to 250 characters if limit is not available From bb523aa5765fd100dd2ab7b8c3b239fc653757c8 Mon Sep 17 00:00:00 2001 From: bastienenjalbert Date: Thu, 22 May 2025 14:54:31 +0200 Subject: [PATCH 3/6] fix(logger) mlflow long parameters logger --- src/lightning/pytorch/loggers/mlflow.py | 5 ++--- tests/tests_pytorch/loggers/test_mlflow.py | 15 +++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/lightning/pytorch/loggers/mlflow.py b/src/lightning/pytorch/loggers/mlflow.py index 620e5742a1f4d..780e6aa442e82 100644 --- a/src/lightning/pytorch/loggers/mlflow.py +++ b/src/lightning/pytorch/loggers/mlflow.py @@ -235,10 +235,9 @@ def log_hyperparams(self, params: Union[dict[str, Any], Namespace]) -> None: import mlflow.utils.validation from mlflow.entities import Param - # Check maximum param value length is available and use it - if hasattr(mlflow.utils.validation, "MAX_PARAM_VAL_LENGTH"): + try: # Check maximum param value length is available and use it param_length_limit = mlflow.utils.validation.MAX_PARAM_VAL_LENGTH - else: # Fallback + except: # Fallback param_length_limit = 250 # Historical default value # Use mlflow default limit or truncate parameter values to 250 characters if limit is not available diff --git a/tests/tests_pytorch/loggers/test_mlflow.py b/tests/tests_pytorch/loggers/test_mlflow.py index c7f9dbe1fe2c6..e0e07c6acb268 100644 --- a/tests/tests_pytorch/loggers/test_mlflow.py +++ b/tests/tests_pytorch/loggers/test_mlflow.py @@ -252,6 +252,14 @@ def test_mlflow_logger_experiment_calls(mlflow_mock, tmp_path): ) param.assert_called_with(key="test", value="test_param") + long_params = {"test": "test_param" * 50} + logger.log_hyperparams(long_params) + + logger.experiment.log_batch.assert_called_with( + run_id=logger.run_id, params=[param(key="test", value="test_param" * 50)] + ) + param.assert_called_with(key="test", value="test_param" * 50) + metrics = {"some_metric": 10} logger.log_metrics(metrics) @@ -317,12 +325,7 @@ def test_mlflow_logger_no_synchronous_support(mlflow_mock, tmp_path): @mock.patch("lightning.pytorch.loggers.mlflow._get_resolve_tags", Mock()) def test_mlflow_logger_with_long_param_value(mlflow_mock, tmp_path): - """Test that long parameter values are truncated to 250 characters.""" - - def _check_value_length(value, *args, **kwargs): - assert len(value) <= 250 - - mlflow_mock.entities.Param.side_effect = _check_value_length + """Test that long parameter values are handled correctly.""" logger = MLFlowLogger("test", save_dir=str(tmp_path)) From 9595a764c9d719e5629bd19a00fb50cfe04c9394 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 22 May 2025 12:54:54 +0000 Subject: [PATCH 4/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/lightning/pytorch/loggers/mlflow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lightning/pytorch/loggers/mlflow.py b/src/lightning/pytorch/loggers/mlflow.py index 780e6aa442e82..c7e2f5546d4ca 100644 --- a/src/lightning/pytorch/loggers/mlflow.py +++ b/src/lightning/pytorch/loggers/mlflow.py @@ -235,7 +235,7 @@ def log_hyperparams(self, params: Union[dict[str, Any], Namespace]) -> None: import mlflow.utils.validation from mlflow.entities import Param - try: # Check maximum param value length is available and use it + try: # Check maximum param value length is available and use it param_length_limit = mlflow.utils.validation.MAX_PARAM_VAL_LENGTH except: # Fallback param_length_limit = 250 # Historical default value From ec33b518bc46e57410a709a2e770942908a94ed8 Mon Sep 17 00:00:00 2001 From: bastienenjalbert Date: Thu, 22 May 2025 14:57:45 +0200 Subject: [PATCH 5/6] fix(logger) issue because of bare except --- src/lightning/pytorch/loggers/mlflow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lightning/pytorch/loggers/mlflow.py b/src/lightning/pytorch/loggers/mlflow.py index c7e2f5546d4ca..392fc7eeedf9d 100644 --- a/src/lightning/pytorch/loggers/mlflow.py +++ b/src/lightning/pytorch/loggers/mlflow.py @@ -237,7 +237,7 @@ def log_hyperparams(self, params: Union[dict[str, Any], Namespace]) -> None: try: # Check maximum param value length is available and use it param_length_limit = mlflow.utils.validation.MAX_PARAM_VAL_LENGTH - except: # Fallback + except Exception as e: # Fallback (in case of MAX_PARAM_VAL_LENGTH not available) param_length_limit = 250 # Historical default value # Use mlflow default limit or truncate parameter values to 250 characters if limit is not available From 43dd53f8e8e14f217c3186ad65fa0926cb6b0bad Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 22 May 2025 12:59:18 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/lightning/pytorch/loggers/mlflow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lightning/pytorch/loggers/mlflow.py b/src/lightning/pytorch/loggers/mlflow.py index 392fc7eeedf9d..e2697da915fbf 100644 --- a/src/lightning/pytorch/loggers/mlflow.py +++ b/src/lightning/pytorch/loggers/mlflow.py @@ -237,7 +237,7 @@ def log_hyperparams(self, params: Union[dict[str, Any], Namespace]) -> None: try: # Check maximum param value length is available and use it param_length_limit = mlflow.utils.validation.MAX_PARAM_VAL_LENGTH - except Exception as e: # Fallback (in case of MAX_PARAM_VAL_LENGTH not available) + except Exception: # Fallback (in case of MAX_PARAM_VAL_LENGTH not available) param_length_limit = 250 # Historical default value # Use mlflow default limit or truncate parameter values to 250 characters if limit is not available