From 670e92d7954f06a7dd2b35396cfdfce1bd9518d5 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Mon, 9 May 2022 14:42:57 -0700 Subject: [PATCH 1/9] add support for MLFLOW_FLATTEN_PARAMS --- src/transformers/integrations.py | 6 +++++- src/transformers/utils/__init__.py | 4 +++- src/transformers/utils/py_utils.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 src/transformers/utils/py_utils.py diff --git a/src/transformers/integrations.py b/src/transformers/integrations.py index 971d55da9041..640bc23bb568 100644 --- a/src/transformers/integrations.py +++ b/src/transformers/integrations.py @@ -23,7 +23,7 @@ import tempfile from pathlib import Path -from .utils import is_datasets_available, logging +from .utils import is_datasets_available, logging, flatten_dict logger = logging.get_logger(__name__) @@ -802,10 +802,13 @@ def setup(self, args, state, model): Allow to reattach to an existing run which can be usefull when resuming training from a checkpoint. When MLFLOW_RUN_ID environment variable is set, start_run attempts to resume a run with the specified run ID and other parameters are ignored. + MLFLOW_FLATTEN_PARAMS (`str`, *optional*): + Whether to flatten the parameters dictionary before logging. Default to `False`. """ self._log_artifacts = os.getenv("HF_MLFLOW_LOG_ARTIFACTS", "FALSE").upper() in ENV_VARS_TRUE_VALUES self._nested_run = os.getenv("MLFLOW_NESTED_RUN", "FALSE").upper() in ENV_VARS_TRUE_VALUES self._experiment_name = os.getenv("MLFLOW_EXPERIMENT_NAME", None) + self._flatten_params = os.getenv("MLFLOW_FLATTEN_PARAMS", "FALSE").upper() in ENV_VARS_TRUE_VALUES self._run_id = os.getenv("MLFLOW_RUN_ID", None) logger.debug( f"MLflow experiment_name={self._experiment_name}, run_name={args.run_name}, nested={self._nested_run}, tags={self._nested_run}" @@ -822,6 +825,7 @@ def setup(self, args, state, model): if hasattr(model, "config") and model.config is not None: model_config = model.config.to_dict() combined_dict = {**model_config, **combined_dict} + combined_dict = flatten_dict(combined_dict) if self._flatten_params else combined_dict # remove params that are too long for MLflow for name, value in list(combined_dict.items()): # internally, all values are converted to str in MLflow diff --git a/src/transformers/utils/__init__.py b/src/transformers/utils/__init__.py index 2c473b389d4e..515ca4c25533 100644 --- a/src/transformers/utils/__init__.py +++ b/src/transformers/utils/__init__.py @@ -137,7 +137,9 @@ torch_required, torch_version, ) - +from .py_utils import ( + flatten_dict, +) WEIGHTS_NAME = "pytorch_model.bin" WEIGHTS_INDEX_NAME = "pytorch_model.bin.index.json" diff --git a/src/transformers/utils/py_utils.py b/src/transformers/utils/py_utils.py new file mode 100644 index 000000000000..f455a93d4870 --- /dev/null +++ b/src/transformers/utils/py_utils.py @@ -0,0 +1,29 @@ +# coding=utf-8 +# Copyright 2021 The HuggingFace Team. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Python utilities""" + +from collections.abc import MutableMapping + + +def flatten_dict(d: MutableMapping, parent_key: str = '', delimiter: str = '.'): + """Flatten a nested dict into a single level dict.""" + def _flatten_dict(d, parent_key="", delimiter="."): + for k, v in d.items(): + key = parent_key + delimiter + k if parent_key else k + if v and isinstance(v, MutableMapping): + yield from flatten_dict(v, key, delimiter=delimiter).items() + else: + yield key, v + return dict(_flatten_dict(d, parent_key, delimiter)) \ No newline at end of file From 46716623921d53d7f7ac809cf6c874c1067b6867 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Mon, 9 May 2022 14:56:44 -0700 Subject: [PATCH 2/9] ensure key is str --- src/transformers/utils/py_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transformers/utils/py_utils.py b/src/transformers/utils/py_utils.py index f455a93d4870..43d6abe70ac2 100644 --- a/src/transformers/utils/py_utils.py +++ b/src/transformers/utils/py_utils.py @@ -21,7 +21,7 @@ def flatten_dict(d: MutableMapping, parent_key: str = '', delimiter: str = '.'): """Flatten a nested dict into a single level dict.""" def _flatten_dict(d, parent_key="", delimiter="."): for k, v in d.items(): - key = parent_key + delimiter + k if parent_key else k + key = str(parent_key) + delimiter + str(k) if parent_key else k if v and isinstance(v, MutableMapping): yield from flatten_dict(v, key, delimiter=delimiter).items() else: From 337ac5d23d02ad2e7f2b069a298d78564416b327 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Mon, 9 May 2022 15:11:19 -0700 Subject: [PATCH 3/9] fix style and update warning msg --- src/transformers/integrations.py | 15 ++++++--------- src/transformers/utils/__init__.py | 5 ++--- src/transformers/utils/py_utils.py | 6 ++++-- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/transformers/integrations.py b/src/transformers/integrations.py index 640bc23bb568..54059be34d3f 100644 --- a/src/transformers/integrations.py +++ b/src/transformers/integrations.py @@ -23,7 +23,7 @@ import tempfile from pathlib import Path -from .utils import is_datasets_available, logging, flatten_dict +from .utils import flatten_dict, is_datasets_available, logging logger = logging.get_logger(__name__) @@ -831,10 +831,9 @@ def setup(self, args, state, model): # internally, all values are converted to str in MLflow if len(str(value)) > self._MAX_PARAM_VAL_LENGTH: logger.warning( - f"Trainer is attempting to log a value of " - f'"{value}" for key "{name}" as a parameter. ' - f"MLflow's log_param() only accepts values no longer than " - f"250 characters so we dropped this attribute." + f'Trainer is attempting to log a value of "{value}" for key "{name}" as a parameter. ' + f"MLflow's log_param() only accepts values no longer than 250 characters so we dropped this attribute. " + f"You can use `MLFLOW_FLATTEN_PARAMS` environment variable to flatten the parameters and avoid this message." ) del combined_dict[name] # MLflow cannot log more than 100 values in one go, so we have to split it @@ -861,10 +860,8 @@ def on_log(self, args, state, control, logs, model=None, **kwargs): metrics[k] = v else: logger.warning( - f"Trainer is attempting to log a value of " - f'"{v}" of type {type(v)} for key "{k}" as a metric. ' - f"MLflow's log_metric() only accepts float and " - f"int types so we dropped this attribute." + f'Trainer is attempting to log a value of "{v}" of type {type(v)} for key "{k}" as a metric. ' + f"MLflow's log_metric() only accepts float and int types so we dropped this attribute." ) self._ml_flow.log_metrics(metrics=metrics, step=state.global_step) diff --git a/src/transformers/utils/__init__.py b/src/transformers/utils/__init__.py index 515ca4c25533..72399b322e7b 100644 --- a/src/transformers/utils/__init__.py +++ b/src/transformers/utils/__init__.py @@ -137,9 +137,8 @@ torch_required, torch_version, ) -from .py_utils import ( - flatten_dict, -) +from .py_utils import flatten_dict + WEIGHTS_NAME = "pytorch_model.bin" WEIGHTS_INDEX_NAME = "pytorch_model.bin.index.json" diff --git a/src/transformers/utils/py_utils.py b/src/transformers/utils/py_utils.py index 43d6abe70ac2..51c434231bd9 100644 --- a/src/transformers/utils/py_utils.py +++ b/src/transformers/utils/py_utils.py @@ -17,8 +17,9 @@ from collections.abc import MutableMapping -def flatten_dict(d: MutableMapping, parent_key: str = '', delimiter: str = '.'): +def flatten_dict(d: MutableMapping, parent_key: str = "", delimiter: str = "."): """Flatten a nested dict into a single level dict.""" + def _flatten_dict(d, parent_key="", delimiter="."): for k, v in d.items(): key = str(parent_key) + delimiter + str(k) if parent_key else k @@ -26,4 +27,5 @@ def _flatten_dict(d, parent_key="", delimiter="."): yield from flatten_dict(v, key, delimiter=delimiter).items() else: yield key, v - return dict(_flatten_dict(d, parent_key, delimiter)) \ No newline at end of file + + return dict(_flatten_dict(d, parent_key, delimiter)) From 8bf593d2ab1886052383c313ebfe943d64932581 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Mon, 9 May 2022 16:20:55 -0700 Subject: [PATCH 4/9] Empty commit to trigger CI From 8ff4afc9e72a5fe4d3399cac08b6aa4cbd64dbcf Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Mon, 9 May 2022 16:43:51 -0700 Subject: [PATCH 5/9] fix bug in check_inits.py --- utils/check_inits.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/check_inits.py b/utils/check_inits.py index 99700ccec598..5b95e609e18d 100644 --- a/utils/check_inits.py +++ b/utils/check_inits.py @@ -249,7 +249,7 @@ def get_transformers_submodules(): if fname == "__init__.py": continue short_path = str((Path(path) / fname).relative_to(PATH_TO_TRANSFORMERS)) - submodule = short_path.replace(os.path.sep, ".").replace(".py", "") + submodule = short_path.replace(".py", "").replace(os.path.sep, ".") if len(submodule.split(".")) == 1: submodules.append(submodule) return submodules From 7c1939cefc69175a02fcb31c3e454f6935cd62b8 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Tue, 10 May 2022 09:04:03 -0700 Subject: [PATCH 6/9] add unittest for flatten_dict utils --- src/transformers/utils/__init__.py | 2 +- src/transformers/utils/generic.py | 15 ++++++++++ src/transformers/utils/py_utils.py | 31 ------------------- tests/utils/test_generics.py | 48 ++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 32 deletions(-) delete mode 100644 src/transformers/utils/py_utils.py create mode 100644 tests/utils/test_generics.py diff --git a/src/transformers/utils/__init__.py b/src/transformers/utils/__init__.py index 72399b322e7b..88891365f03e 100644 --- a/src/transformers/utils/__init__.py +++ b/src/transformers/utils/__init__.py @@ -38,6 +38,7 @@ TensorType, cached_property, find_labels, + flatten_dict, is_tensor, to_numpy, to_py_obj, @@ -137,7 +138,6 @@ torch_required, torch_version, ) -from .py_utils import flatten_dict WEIGHTS_NAME = "pytorch_model.bin" diff --git a/src/transformers/utils/generic.py b/src/transformers/utils/generic.py index bea5b3dd4775..136762a37858 100644 --- a/src/transformers/utils/generic.py +++ b/src/transformers/utils/generic.py @@ -17,6 +17,7 @@ import inspect from collections import OrderedDict, UserDict +from collections.abc import MutableMapping from contextlib import ExitStack from dataclasses import fields from enum import Enum @@ -310,3 +311,17 @@ def find_labels(model_class): return [p for p in signature.parameters if "label" in p or p in ("start_positions", "end_positions")] else: return [p for p in signature.parameters if "label" in p] + + +def flatten_dict(d: MutableMapping, parent_key: str = "", delimiter: str = "."): + """Flatten a nested dict into a single level dict.""" + + def _flatten_dict(d, parent_key="", delimiter="."): + for k, v in d.items(): + key = str(parent_key) + delimiter + str(k) if parent_key else k + if v and isinstance(v, MutableMapping): + yield from flatten_dict(v, key, delimiter=delimiter).items() + else: + yield key, v + + return dict(_flatten_dict(d, parent_key, delimiter)) diff --git a/src/transformers/utils/py_utils.py b/src/transformers/utils/py_utils.py deleted file mode 100644 index 51c434231bd9..000000000000 --- a/src/transformers/utils/py_utils.py +++ /dev/null @@ -1,31 +0,0 @@ -# coding=utf-8 -# Copyright 2021 The HuggingFace Team. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Python utilities""" - -from collections.abc import MutableMapping - - -def flatten_dict(d: MutableMapping, parent_key: str = "", delimiter: str = "."): - """Flatten a nested dict into a single level dict.""" - - def _flatten_dict(d, parent_key="", delimiter="."): - for k, v in d.items(): - key = str(parent_key) + delimiter + str(k) if parent_key else k - if v and isinstance(v, MutableMapping): - yield from flatten_dict(v, key, delimiter=delimiter).items() - else: - yield key, v - - return dict(_flatten_dict(d, parent_key, delimiter)) diff --git a/tests/utils/test_generics.py b/tests/utils/test_generics.py new file mode 100644 index 000000000000..f0a349c65c7d --- /dev/null +++ b/tests/utils/test_generics.py @@ -0,0 +1,48 @@ +# coding=utf-8 +# Copyright 2019-present, the HuggingFace Inc. team. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +from transformers.utils import flatten_dict + + +class GenericsTester(unittest.TestCase): + def test_flatten_dict(self): + input_dict = { + "task_specific_params": { + "summarization": {"length_penalty": 1.0, "max_length": 128, "min_length": 12, "num_beams": 4}, + "summarization_cnn": {"length_penalty": 2.0, "max_length": 142, "min_length": 56, "num_beams": 4}, + "summarization_xsum": {"length_penalty": 1.0, "max_length": 62, "min_length": 11, "num_beams": 6}, + } + } + expected_dict = { + "task_specific_params.summarization.length_penalty": 1.0, + "task_specific_params.summarization.max_length": 128, + "task_specific_params.summarization.min_length": 12, + "task_specific_params.summarization.num_beams": 4, + "task_specific_params.summarization_cnn.length_penalty": 2.0, + "task_specific_params.summarization_cnn.max_length": 142, + "task_specific_params.summarization_cnn.min_length": 56, + "task_specific_params.summarization_cnn.num_beams": 4, + "task_specific_params.summarization_xsum.length_penalty": 1.0, + "task_specific_params.summarization_xsum.max_length": 62, + "task_specific_params.summarization_xsum.min_length": 11, + "task_specific_params.summarization_xsum.num_beams": 6, + } + + self.assertEqual( + flatten_dict(input_dict), + expected_dict, + ) From 779dc4ec9d7e23b1cc186afe5c236c111ac36aa0 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Tue, 10 May 2022 09:05:24 -0700 Subject: [PATCH 7/9] fix 'NoneType' object is not callable on __del__ --- src/transformers/integrations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transformers/integrations.py b/src/transformers/integrations.py index 54059be34d3f..625ee6875a6c 100644 --- a/src/transformers/integrations.py +++ b/src/transformers/integrations.py @@ -876,7 +876,7 @@ def on_train_end(self, args, state, control, **kwargs): def __del__(self): # if the previous run is not terminated correctly, the fluent API will # not let you start a new run before the previous one is killed - if self._auto_end_run and self._ml_flow.active_run() is not None: + if self._auto_end_run and self._ml_flow and self._ml_flow.active_run() is not None: self._ml_flow.end_run() From a00bcd8f52a51bfc6905ae077285eec32337cf20 Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Tue, 10 May 2022 09:24:28 -0700 Subject: [PATCH 8/9] add generic flatten_dict unittest to SPECIAL_MODULE_TO_TEST_MAP --- tests/utils/{test_generics.py => test_generic.py} | 2 +- utils/tests_fetcher.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename tests/utils/{test_generics.py => test_generic.py} (98%) diff --git a/tests/utils/test_generics.py b/tests/utils/test_generic.py similarity index 98% rename from tests/utils/test_generics.py rename to tests/utils/test_generic.py index f0a349c65c7d..074582d89d78 100644 --- a/tests/utils/test_generics.py +++ b/tests/utils/test_generic.py @@ -18,7 +18,7 @@ from transformers.utils import flatten_dict -class GenericsTester(unittest.TestCase): +class GenericTester(unittest.TestCase): def test_flatten_dict(self): input_dict = { "task_specific_params": { diff --git a/utils/tests_fetcher.py b/utils/tests_fetcher.py index f733f301e8dc..1eda2be47f57 100644 --- a/utils/tests_fetcher.py +++ b/utils/tests_fetcher.py @@ -268,7 +268,7 @@ def create_reverse_dependency_map(): "feature_extraction_sequence_utils.py": "test_sequence_feature_extraction_common.py", "feature_extraction_utils.py": "test_feature_extraction_common.py", "file_utils.py": ["utils/test_file_utils.py", "utils/test_model_output.py"], - "utils/generic.py": ["utils/test_file_utils.py", "utils/test_model_output.py"], + "utils/generic.py": ["utils/test_file_utils.py", "utils/test_model_output.py", "utils/test_generic.py"], "utils/hub.py": "utils/test_file_utils.py", "modelcard.py": "utils/test_model_card.py", "modeling_flax_utils.py": "test_modeling_flax_common.py", From c2aab41575fa18cb5d9e82ffb384390716a6220a Mon Sep 17 00:00:00 2001 From: Nicolas Brousse Date: Tue, 10 May 2022 09:28:37 -0700 Subject: [PATCH 9/9] fix style --- tests/utils/test_generic.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/utils/test_generic.py b/tests/utils/test_generic.py index 074582d89d78..6fbdbee40360 100644 --- a/tests/utils/test_generic.py +++ b/tests/utils/test_generic.py @@ -42,7 +42,4 @@ def test_flatten_dict(self): "task_specific_params.summarization_xsum.num_beams": 6, } - self.assertEqual( - flatten_dict(input_dict), - expected_dict, - ) + self.assertEqual(flatten_dict(input_dict), expected_dict)