From 3c8718ce85a4cbbbf559b038d40f69310a857f9e Mon Sep 17 00:00:00 2001 From: Joao Gante Date: Thu, 3 Oct 2024 09:54:45 +0000 Subject: [PATCH 1/4] lower to warning --- src/transformers/configuration_utils.py | 5 +++-- tests/utils/test_configuration_utils.py | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/transformers/configuration_utils.py b/src/transformers/configuration_utils.py index 92e5425e95e7..c6d7fc025681 100755 --- a/src/transformers/configuration_utils.py +++ b/src/transformers/configuration_utils.py @@ -380,11 +380,12 @@ def save_pretrained(self, save_directory: Union[str, os.PathLike], push_to_hub: non_default_generation_parameters = self._get_non_default_generation_parameters() if len(non_default_generation_parameters) > 0: - raise ValueError( + # TODO (joao): this should be an exception if the user has modified the loaded config. See #33886 + warnings.warn( "Some non-default generation parameters are set in the model config. These should go into either a) " "`model.generation_config` (as opposed to `model.config`); OR b) a GenerationConfig file " "(https://huggingface.co/docs/transformers/generation_strategies#save-a-custom-decoding-strategy-with-your-model) " - f"\nNon-default generation parameters: {str(non_default_generation_parameters)}" + f"\nNon-default generation parameters: {str(non_default_generation_parameters)}", UserWarning ) os.makedirs(save_directory, exist_ok=True) diff --git a/tests/utils/test_configuration_utils.py b/tests/utils/test_configuration_utils.py index 76394daf9ced..d2701bf35e66 100644 --- a/tests/utils/test_configuration_utils.py +++ b/tests/utils/test_configuration_utils.py @@ -313,11 +313,12 @@ def test_repo_versioning_before(self): old_configuration = old_transformers.models.auto.AutoConfig.from_pretrained(repo) self.assertEqual(old_configuration.hidden_size, 768) - def test_saving_config_with_custom_generation_kwargs_raises_exception(self): + def test_saving_config_with_custom_generation_kwargs_raises_warning(self): config = BertConfig(min_length=3) # `min_length = 3` is a non-default generation kwarg with tempfile.TemporaryDirectory() as tmp_dir: - with self.assertRaises(ValueError): + with self.assertWarns(UserWarning) as cm: config.save_pretrained(tmp_dir) + self.assertIn("min_length", str(cm.warning)) def test_get_non_default_generation_parameters(self): config = BertConfig() From d5cc337fc38df9cde0590b66e8d07d7bafe477ef Mon Sep 17 00:00:00 2001 From: Joao Gante Date: Thu, 3 Oct 2024 09:55:12 +0000 Subject: [PATCH 2/4] msg --- src/transformers/configuration_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/transformers/configuration_utils.py b/src/transformers/configuration_utils.py index c6d7fc025681..a465a2f320d8 100755 --- a/src/transformers/configuration_utils.py +++ b/src/transformers/configuration_utils.py @@ -384,7 +384,8 @@ def save_pretrained(self, save_directory: Union[str, os.PathLike], push_to_hub: warnings.warn( "Some non-default generation parameters are set in the model config. These should go into either a) " "`model.generation_config` (as opposed to `model.config`); OR b) a GenerationConfig file " - "(https://huggingface.co/docs/transformers/generation_strategies#save-a-custom-decoding-strategy-with-your-model) " + "(https://huggingface.co/docs/transformers/generation_strategies#save-a-custom-decoding-strategy-with-your-model)." + "This warning will become an exception in the future.", f"\nNon-default generation parameters: {str(non_default_generation_parameters)}", UserWarning ) From 5034481a99f92785e0f1b71d176a98440cda2e34 Mon Sep 17 00:00:00 2001 From: Joao Gante Date: Thu, 3 Oct 2024 10:02:07 +0000 Subject: [PATCH 3/4] make fixup --- src/transformers/configuration_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/transformers/configuration_utils.py b/src/transformers/configuration_utils.py index a465a2f320d8..1bbb9b59e1d2 100755 --- a/src/transformers/configuration_utils.py +++ b/src/transformers/configuration_utils.py @@ -386,7 +386,8 @@ def save_pretrained(self, save_directory: Union[str, os.PathLike], push_to_hub: "`model.generation_config` (as opposed to `model.config`); OR b) a GenerationConfig file " "(https://huggingface.co/docs/transformers/generation_strategies#save-a-custom-decoding-strategy-with-your-model)." "This warning will become an exception in the future.", - f"\nNon-default generation parameters: {str(non_default_generation_parameters)}", UserWarning + f"\nNon-default generation parameters: {str(non_default_generation_parameters)}", + UserWarning, ) os.makedirs(save_directory, exist_ok=True) From cc98529925764e89e82ed643178d2d8f70913cb3 Mon Sep 17 00:00:00 2001 From: Joao Gante Date: Thu, 3 Oct 2024 11:06:33 +0000 Subject: [PATCH 4/4] rm extra comma --- src/transformers/configuration_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transformers/configuration_utils.py b/src/transformers/configuration_utils.py index 1bbb9b59e1d2..8d8784be3911 100755 --- a/src/transformers/configuration_utils.py +++ b/src/transformers/configuration_utils.py @@ -385,7 +385,7 @@ def save_pretrained(self, save_directory: Union[str, os.PathLike], push_to_hub: "Some non-default generation parameters are set in the model config. These should go into either a) " "`model.generation_config` (as opposed to `model.config`); OR b) a GenerationConfig file " "(https://huggingface.co/docs/transformers/generation_strategies#save-a-custom-decoding-strategy-with-your-model)." - "This warning will become an exception in the future.", + "This warning will become an exception in the future." f"\nNon-default generation parameters: {str(non_default_generation_parameters)}", UserWarning, )