Possible bug in `forced_decoder_ids` in `modeling_whisper.py`

[akuzeee]

System Info

N/A

Who can help?

@sanchit-gandhi @connor-henderson

Hi, thank you for the great work!

transformers/src/transformers/models/whisper/modeling_whisper.py

Line 1649 in 796162c

forced_decoder_ids = [

might have a bug (which is introduced in this PR).

According to a comparison with openAI's whisper, I think the order of the elements should be as follows:

            forced_decoder_ids = [
                # Slicing the text prompt ids in a manner consistent with the OpenAI implementation
                # to accomodate context space for the prefix (see https://github.com/openai/whisper/blob/c09a7ae299c4c34c5839a76380ae407e7d785914/whisper/decoding.py#L599)
                generation_config.decoder_start_token_id,
                *text_prompt_ids[-self.config.max_length // 2 - 1 :],
                *[token for _rank, token in non_prompt_forced_decoder_ids],
            ]

Information

• The official example scripts
• My own modified scripts

Tasks

• An officially supported task in the examples folder (such as GLUE/SQuAD, ...)
• My own task or dataset (give details below)

Reproduction

N/A

Expected behavior

N/A

[sanchit-gandhi]

Hey @akuzeee! Cool to see you've been going through the Transformers Whisper codebase! We actually update the decoder start token id prior to setting the forced decoder ids:

transformers/src/transformers/models/whisper/modeling_whisper.py

Lines 1636 to 1637 in 796162c

	# Set the decoder_start_token_id to <|startofprev|>
	kwargs.update({"decoder_start_token_id": decoder_start_token_id})

The decoder start token id is set to the <|startofprev|> token (or self.tokenizer.sot_prev in the Whisper codebase), and is always forced as the beginning of sequence (BOS) token during generation (i.e. the first token in our sequence). So the decoder start token id is handled outside of the forced decoder ids.

We then add the <|startoftranscript|> token after the prompts, which indicates the end of the prompt and start of the generation. It seems like the OpenAI codebase has this <|startoftranscript|> token included as part of their tokens, i.e. looking at the code from https://github.com/openai/whisper/blob/c09a7ae299c4c34c5839a76380ae407e7d785914/whisper/decoding.py#L597:

            tokens = (
                [self.tokenizer.sot_prev]  # this is equivalent to us setting the sot prev start token id as the decoder start token id
                + prompt_tokens[-(self.n_ctx // 2 - 1) :]  # this is the same as our code
                + tokens  # these tokens include the bos decoder start token id
            )

[akuzeee]

Thank you for the detailed and kind explanation! I understand now.