From 389f35b2541f06b72edc94d986643f948169de4b Mon Sep 17 00:00:00 2001 From: "arthur.zucker@gmail.com" Date: Fri, 3 Mar 2023 18:15:09 +0000 Subject: [PATCH 1/8] temp fix --- .../models/whisper/modeling_whisper.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/transformers/models/whisper/modeling_whisper.py b/src/transformers/models/whisper/modeling_whisper.py index 51fffe001dc2..e0f87b36d6a9 100644 --- a/src/transformers/models/whisper/modeling_whisper.py +++ b/src/transformers/models/whisper/modeling_whisper.py @@ -14,7 +14,7 @@ # limitations under the License. """ PyTorch Whisper model.""" - +import warnings import math import random from typing import Optional, Tuple, Union @@ -1414,7 +1414,7 @@ def generate( stopping_criteria=None, prefix_allowed_tokens_fn=None, synced_gpus=False, - return_timestamps=None, + return_timestamps=False, task=None, language=None, is_multilingual=None, @@ -1515,6 +1515,7 @@ def generate( if language is not None: generation_config.language = language + forced_decoder_ids = [] if hasattr(generation_config, "is_multilingual") and generation_config.is_multilingual: @@ -1528,6 +1529,17 @@ def generate( else: forced_decoder_ids.append((2, generation_config.task_to_id["transcribe"])) + + # Legacy code for backward compatibility + if forced_decoder_ids != self.config.forced_decoder_ids and forced_decoder_ids is not None: + warnings.warn( + "You have modified the pretrained model configuration to control generation. This is a" + " deprecated strategy to control generation and will be removed soon, in a future version." + " Please use a generation configuration file (see" + " https://huggingface.co/docs/transformers/main_classes/text_generation)" + ) + forced_decoder_ids = self.config.forced_decoder_ids + if ( hasattr(generation_config, "return_timestamps") and generation_config.return_timestamps ) or return_timestamps: From 10a217557a473ac6a93013fc221f1a003f5a411d Mon Sep 17 00:00:00 2001 From: "arthur.zucker@gmail.com" Date: Mon, 6 Mar 2023 10:19:47 +0000 Subject: [PATCH 2/8] temporary fix --- src/transformers/models/whisper/modeling_whisper.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/transformers/models/whisper/modeling_whisper.py b/src/transformers/models/whisper/modeling_whisper.py index e0f87b36d6a9..43eb19c3b3dc 100644 --- a/src/transformers/models/whisper/modeling_whisper.py +++ b/src/transformers/models/whisper/modeling_whisper.py @@ -1504,11 +1504,12 @@ def generate( generation_config = self.generation_config if return_timestamps is not None: + # legacy check for models that do not have a generation config + if not hasattr(generation_config, "no_timestamps_token_id"): + raise ValueError("You are trying to return timestamps, but the generation config is not properly set." + "Make sure to initialize the generation config with the correct `no_timestamps_token_id`.") generation_config.return_timestamps = return_timestamps - if task is not None: - generation_config.task = task - if is_multilingual is not None: generation_config.is_multilingual = is_multilingual @@ -1540,11 +1541,11 @@ def generate( ) forced_decoder_ids = self.config.forced_decoder_ids - if ( - hasattr(generation_config, "return_timestamps") and generation_config.return_timestamps - ) or return_timestamps: + if generation_config.return_timestamps: logits_processor = [WhisperTimeStampLogitsProcessor(generation_config)] else: + if not hasattr(generation_config,"no_timestamps_token_id"): + generation_config.no_timestamp_token_id = 50363 + 1 * () if forced_decoder_ids and forced_decoder_ids[-1][0] != generation_config.no_timestamps_token_id: idx = forced_decoder_ids[-1][0] + 1 if forced_decoder_ids else 1 forced_decoder_ids.append((idx, generation_config.no_timestamps_token_id)) From 25c07110632b3481268545a71acbbf15cc6dc26a Mon Sep 17 00:00:00 2001 From: "arthur.zucker@gmail.com" Date: Mon, 6 Mar 2023 13:24:22 +0000 Subject: [PATCH 3/8] update --- .../models/whisper/modeling_whisper.py | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/transformers/models/whisper/modeling_whisper.py b/src/transformers/models/whisper/modeling_whisper.py index 43eb19c3b3dc..8916d1cd0c1e 100644 --- a/src/transformers/models/whisper/modeling_whisper.py +++ b/src/transformers/models/whisper/modeling_whisper.py @@ -14,9 +14,9 @@ # limitations under the License. """ PyTorch Whisper model.""" -import warnings import math import random +import warnings from typing import Optional, Tuple, Union import numpy as np @@ -36,6 +36,7 @@ from ...modeling_utils import PreTrainedModel from ...utils import add_start_docstrings, add_start_docstrings_to_model_forward, logging, replace_return_docstrings from .configuration_whisper import WhisperConfig +from .tokenization_whisper import TASK_IDS, TO_LANGUAGE_CODE logger = logging.get_logger(__name__) @@ -1504,10 +1505,13 @@ def generate( generation_config = self.generation_config if return_timestamps is not None: - # legacy check for models that do not have a generation config if not hasattr(generation_config, "no_timestamps_token_id"): - raise ValueError("You are trying to return timestamps, but the generation config is not properly set." - "Make sure to initialize the generation config with the correct `no_timestamps_token_id`.") + raise ValueError( + "You are trying to return timestamps, but the generation config is not properly set." + "Make sure to initialize the generation config with the correct `no_timestamps_token_id`." + "For more details on how to generate the approtiate config, refer to [#21878](https://github.com/huggingface/transformers/issues/21878)" + ) + generation_config.return_timestamps = return_timestamps if is_multilingual is not None: @@ -1516,23 +1520,32 @@ def generate( if language is not None: generation_config.language = language - forced_decoder_ids = [] if hasattr(generation_config, "is_multilingual") and generation_config.is_multilingual: if hasattr(generation_config, "language"): - forced_decoder_ids.append((1, generation_config.lang_to_id[generation_config.language])) + if generation_config.language in generation_config.lang_to_id.keys(): + language_token = generation_config.language + elif generation_config.language in TO_LANGUAGE_CODE.keys(): + language_token = f"<|{TO_LANGUAGE_CODE[generation_config.language]}|>" + else: + raise ValueError(f"The `{generation_config.language}` language token is not supported.") + forced_decoder_ids.append((1, generation_config.lang_to_id[language_token])) else: forced_decoder_ids.append((1, None)) if hasattr(generation_config, "task"): - forced_decoder_ids.append((2, generation_config.task_to_id[generation_config.task])) + if generation_config.task in TASK_IDS: + forced_decoder_ids.append((2, generation_config.task_to_id[generation_config.task])) + else: + raise ValueError( + f"The `{generation_config.task}`task is not supported. The task should be one of `{TASK_IDS}`" + ) else: forced_decoder_ids.append((2, generation_config.task_to_id["transcribe"])) - # Legacy code for backward compatibility - if forced_decoder_ids != self.config.forced_decoder_ids and forced_decoder_ids is not None: + if forced_decoder_ids != self.config.forced_decoder_ids: warnings.warn( "You have modified the pretrained model configuration to control generation. This is a" " deprecated strategy to control generation and will be removed soon, in a future version." @@ -1540,12 +1553,10 @@ def generate( " https://huggingface.co/docs/transformers/main_classes/text_generation)" ) forced_decoder_ids = self.config.forced_decoder_ids - + if generation_config.return_timestamps: logits_processor = [WhisperTimeStampLogitsProcessor(generation_config)] else: - if not hasattr(generation_config,"no_timestamps_token_id"): - generation_config.no_timestamp_token_id = 50363 + 1 * () if forced_decoder_ids and forced_decoder_ids[-1][0] != generation_config.no_timestamps_token_id: idx = forced_decoder_ids[-1][0] + 1 if forced_decoder_ids else 1 forced_decoder_ids.append((idx, generation_config.no_timestamps_token_id)) From 2262ca6d5b47c8073d5ded19acae0c31c06d21d3 Mon Sep 17 00:00:00 2001 From: "arthur.zucker@gmail.com" Date: Mon, 6 Mar 2023 15:34:41 +0000 Subject: [PATCH 4/8] fix tests --- src/transformers/models/whisper/modeling_whisper.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/transformers/models/whisper/modeling_whisper.py b/src/transformers/models/whisper/modeling_whisper.py index 8916d1cd0c1e..a83a2f67bf56 100644 --- a/src/transformers/models/whisper/modeling_whisper.py +++ b/src/transformers/models/whisper/modeling_whisper.py @@ -1415,7 +1415,7 @@ def generate( stopping_criteria=None, prefix_allowed_tokens_fn=None, synced_gpus=False, - return_timestamps=False, + return_timestamps=None, task=None, language=None, is_multilingual=None, @@ -1513,7 +1513,8 @@ def generate( ) generation_config.return_timestamps = return_timestamps - + else: + generation_config.return_timestamps = False if is_multilingual is not None: generation_config.is_multilingual = is_multilingual @@ -1545,7 +1546,7 @@ def generate( forced_decoder_ids.append((2, generation_config.task_to_id["transcribe"])) # Legacy code for backward compatibility - if forced_decoder_ids != self.config.forced_decoder_ids: + if hasattr(self.config, "forced_decoder_ids") and forced_decoder_ids != self.generation.forced_decoder_ids: warnings.warn( "You have modified the pretrained model configuration to control generation. This is a" " deprecated strategy to control generation and will be removed soon, in a future version." From ea0eca479e8c9cbc2de8dc1cd6941fbeb2d2dbcb Mon Sep 17 00:00:00 2001 From: "arthur.zucker@gmail.com" Date: Mon, 6 Mar 2023 15:59:09 +0000 Subject: [PATCH 5/8] fixup --- src/transformers/models/whisper/modeling_whisper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transformers/models/whisper/modeling_whisper.py b/src/transformers/models/whisper/modeling_whisper.py index a83a2f67bf56..c5fa83fd07ec 100644 --- a/src/transformers/models/whisper/modeling_whisper.py +++ b/src/transformers/models/whisper/modeling_whisper.py @@ -1546,7 +1546,7 @@ def generate( forced_decoder_ids.append((2, generation_config.task_to_id["transcribe"])) # Legacy code for backward compatibility - if hasattr(self.config, "forced_decoder_ids") and forced_decoder_ids != self.generation.forced_decoder_ids: + if hasattr(self.config, "forced_decoder_ids") and forced_decoder_ids != self.generation.forced_decoder_ids: warnings.warn( "You have modified the pretrained model configuration to control generation. This is a" " deprecated strategy to control generation and will be removed soon, in a future version." From 5d18c67eb3f862fe9c185d13cfa3143d28e70829 Mon Sep 17 00:00:00 2001 From: "arthur.zucker@gmail.com" Date: Wed, 8 Mar 2023 11:33:09 +0000 Subject: [PATCH 6/8] update based on reveiew Co-authored-by: Sanchit Gandhi <93869735+sanchit-gandhi@users.noreply.github.com> --- .../models/whisper/modeling_whisper.py | 64 ++++++++++--------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/src/transformers/models/whisper/modeling_whisper.py b/src/transformers/models/whisper/modeling_whisper.py index c5fa83fd07ec..e23bd7b4055a 100644 --- a/src/transformers/models/whisper/modeling_whisper.py +++ b/src/transformers/models/whisper/modeling_whisper.py @@ -16,7 +16,6 @@ import math import random -import warnings from typing import Optional, Tuple, Union import numpy as np @@ -1509,7 +1508,7 @@ def generate( raise ValueError( "You are trying to return timestamps, but the generation config is not properly set." "Make sure to initialize the generation config with the correct `no_timestamps_token_id`." - "For more details on how to generate the approtiate config, refer to [#21878](https://github.com/huggingface/transformers/issues/21878)" + "For more details on how to generate the approtiate config, refer to https://github.com/huggingface/transformers/issues/21878#issuecomment-1451902363" ) generation_config.return_timestamps = return_timestamps @@ -1520,40 +1519,43 @@ def generate( if language is not None: generation_config.language = language - - forced_decoder_ids = [] - - if hasattr(generation_config, "is_multilingual") and generation_config.is_multilingual: - if hasattr(generation_config, "language"): - if generation_config.language in generation_config.lang_to_id.keys(): - language_token = generation_config.language - elif generation_config.language in TO_LANGUAGE_CODE.keys(): - language_token = f"<|{TO_LANGUAGE_CODE[generation_config.language]}|>" + if task is not None: + generation_config.task = task + + if task is not None or language is not None: + forced_decoder_ids = [] + if hasattr(generation_config, "is_multilingual") and generation_config.is_multilingual: + if hasattr(generation_config, "language"): + if generation_config.language in generation_config.lang_to_id.keys(): + language_token = generation_config.language + elif generation_config.language in TO_LANGUAGE_CODE.keys(): + language_token = f"<|{TO_LANGUAGE_CODE[generation_config.language]}|>" + else: + raise ValueError(f"The `{generation_config.language}` language token is not supported.") + forced_decoder_ids.append((1, generation_config.lang_to_id[language_token])) else: - raise ValueError(f"The `{generation_config.language}` language token is not supported.") - forced_decoder_ids.append((1, generation_config.lang_to_id[language_token])) - else: - forced_decoder_ids.append((1, None)) - - if hasattr(generation_config, "task"): - if generation_config.task in TASK_IDS: - forced_decoder_ids.append((2, generation_config.task_to_id[generation_config.task])) + forced_decoder_ids.append((1, None)) # automatically detect the language + + if hasattr(generation_config, "task"): + if generation_config.task in TASK_IDS: + forced_decoder_ids.append((2, generation_config.task_to_id[generation_config.task])) + else: + raise ValueError( + f"The `{generation_config.task}`task is not supported. The task should be one of `{TASK_IDS}`" + ) else: - raise ValueError( - f"The `{generation_config.task}`task is not supported. The task should be one of `{TASK_IDS}`" - ) - else: - forced_decoder_ids.append((2, generation_config.task_to_id["transcribe"])) + forced_decoder_ids.append( + (2, generation_config.task_to_id["transcribe"]) + ) # defaults to transcribe # Legacy code for backward compatibility - if hasattr(self.config, "forced_decoder_ids") and forced_decoder_ids != self.generation.forced_decoder_ids: - warnings.warn( - "You have modified the pretrained model configuration to control generation. This is a" - " deprecated strategy to control generation and will be removed soon, in a future version." - " Please use a generation configuration file (see" - " https://huggingface.co/docs/transformers/main_classes/text_generation)" - ) + elif hasattr(self.config, "forced_decoder_ids"): forced_decoder_ids = self.config.forced_decoder_ids + else: + raise ValueError( + "No `task` or `languages` were specified, and the `generation_config` does not have these attributes either." + "To use a default language and task, update the `model.generation_config`." + ) if generation_config.return_timestamps: logits_processor = [WhisperTimeStampLogitsProcessor(generation_config)] From a7d706fedf0216d20a50401ef74c20641be0a928 Mon Sep 17 00:00:00 2001 From: "arthur.zucker@gmail.com" Date: Wed, 8 Mar 2023 16:36:55 +0000 Subject: [PATCH 7/8] update to fix tests --- .../models/whisper/modeling_whisper.py | 64 +++++++++---------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/src/transformers/models/whisper/modeling_whisper.py b/src/transformers/models/whisper/modeling_whisper.py index e23bd7b4055a..2937f3e65ac5 100644 --- a/src/transformers/models/whisper/modeling_whisper.py +++ b/src/transformers/models/whisper/modeling_whisper.py @@ -1507,62 +1507,56 @@ def generate( if not hasattr(generation_config, "no_timestamps_token_id"): raise ValueError( "You are trying to return timestamps, but the generation config is not properly set." - "Make sure to initialize the generation config with the correct `no_timestamps_token_id`." + "Make sure to initialize the generation config with the correct attributes that are needed such as `no_timestamps_token_id`." "For more details on how to generate the approtiate config, refer to https://github.com/huggingface/transformers/issues/21878#issuecomment-1451902363" ) generation_config.return_timestamps = return_timestamps else: generation_config.return_timestamps = False - if is_multilingual is not None: - generation_config.is_multilingual = is_multilingual if language is not None: generation_config.language = language if task is not None: generation_config.task = task + forced_decoder_ids = [] if task is not None or language is not None: - forced_decoder_ids = [] - if hasattr(generation_config, "is_multilingual") and generation_config.is_multilingual: - if hasattr(generation_config, "language"): - if generation_config.language in generation_config.lang_to_id.keys(): - language_token = generation_config.language - elif generation_config.language in TO_LANGUAGE_CODE.keys(): - language_token = f"<|{TO_LANGUAGE_CODE[generation_config.language]}|>" - else: - raise ValueError(f"The `{generation_config.language}` language token is not supported.") - forced_decoder_ids.append((1, generation_config.lang_to_id[language_token])) + if hasattr(generation_config, "language"): + if generation_config.language in generation_config.lang_to_id.keys(): + language_token = generation_config.language + elif generation_config.language in TO_LANGUAGE_CODE.keys(): + language_token = f"<|{TO_LANGUAGE_CODE[generation_config.language]}|>" else: - forced_decoder_ids.append((1, None)) # automatically detect the language - - if hasattr(generation_config, "task"): - if generation_config.task in TASK_IDS: - forced_decoder_ids.append((2, generation_config.task_to_id[generation_config.task])) - else: - raise ValueError( - f"The `{generation_config.task}`task is not supported. The task should be one of `{TASK_IDS}`" - ) + raise ValueError(f"The `{generation_config.language}` language token is not supported.") + forced_decoder_ids.append((1, generation_config.lang_to_id[language_token])) + else: + forced_decoder_ids.append((1, None)) # automatically detect the language + + if hasattr(generation_config, "task"): + if generation_config.task in TASK_IDS: + forced_decoder_ids.append((2, generation_config.task_to_id[generation_config.task])) else: - forced_decoder_ids.append( - (2, generation_config.task_to_id["transcribe"]) - ) # defaults to transcribe + raise ValueError( + f"The `{generation_config.task}`task is not supported. The task should be one of `{TASK_IDS}`" + ) + else: + forced_decoder_ids.append((2, generation_config.task_to_id["transcribe"])) # defaults to transcribe + if hasattr(generation_config, "no_timestamps_token_id") and not generation_config.return_timestamps: + idx = forced_decoder_ids[-1][0] + 1 if forced_decoder_ids else 1 + forced_decoder_ids.append((idx, generation_config.no_timestamps_token_id)) # Legacy code for backward compatibility - elif hasattr(self.config, "forced_decoder_ids"): + elif hasattr(self.config, "forced_decoder_ids") and self.config.forced_decoder_ids is not None: forced_decoder_ids = self.config.forced_decoder_ids - else: - raise ValueError( - "No `task` or `languages` were specified, and the `generation_config` does not have these attributes either." - "To use a default language and task, update the `model.generation_config`." - ) + elif ( + hasattr(self.generation_config, "forced_decoder_ids") + and self.generation_config.forced_decoder_ids is not None + ): + forced_decoder_ids = self.generation_config.forced_decoder_ids if generation_config.return_timestamps: logits_processor = [WhisperTimeStampLogitsProcessor(generation_config)] - else: - if forced_decoder_ids and forced_decoder_ids[-1][0] != generation_config.no_timestamps_token_id: - idx = forced_decoder_ids[-1][0] + 1 if forced_decoder_ids else 1 - forced_decoder_ids.append((idx, generation_config.no_timestamps_token_id)) if len(forced_decoder_ids) > 0: generation_config.forced_decoder_ids = forced_decoder_ids From f3c45564912816d36f1b6f7d777376e1f76be1d9 Mon Sep 17 00:00:00 2001 From: "arthur.zucker@gmail.com" Date: Mon, 13 Mar 2023 07:51:02 +0000 Subject: [PATCH 8/8] update docstring --- src/transformers/models/whisper/modeling_whisper.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/transformers/models/whisper/modeling_whisper.py b/src/transformers/models/whisper/modeling_whisper.py index 2937f3e65ac5..849a2a0146c7 100644 --- a/src/transformers/models/whisper/modeling_whisper.py +++ b/src/transformers/models/whisper/modeling_whisper.py @@ -1471,8 +1471,8 @@ def generate( Task to use for generation, either "translate" or "transcribe". The `model.config.forced_decoder_ids` will be updated accordingly. language (`bool`, *optional*): - Language token to use for generation, should be in the form `<|en|>`. You can find all the possible - language tokens in the `model.generation_config.lang_to_id` dictionary. + Language token to use for generation, can be either in the form of `<|en|>`, `en` or `english`. You can + find all the possible language tokens in the `model.generation_config.lang_to_id` dictionary. is_multilingual (`bool`, *optional*): Whether or not the model is multilingual. kwargs: @@ -1528,7 +1528,10 @@ def generate( elif generation_config.language in TO_LANGUAGE_CODE.keys(): language_token = f"<|{TO_LANGUAGE_CODE[generation_config.language]}|>" else: - raise ValueError(f"The `{generation_config.language}` language token is not supported.") + raise ValueError( + f"Unsupported language: {self.language}. Language should be one of:" + f" {list(TO_LANGUAGE_CODE.keys()) if generation_config.language in TO_LANGUAGE_CODE.keys() else list(TO_LANGUAGE_CODE.values())}." + ) forced_decoder_ids.append((1, generation_config.lang_to_id[language_token])) else: forced_decoder_ids.append((1, None)) # automatically detect the language