[Whisper] 🚨 Fix whisper decoding 🚨
Author: eustlb
Pull request: yes
Labels: name: run-slow

# What does this PR do?

This PR finalizes #30984 which enabled short-form (_<=30sec_) and long-form generation using temperature fallback. Indeed, the original OpenAI implementation uses the same [decode_with_fallback](https://github.com/openai/whisper/blob/25639fc17ddc013d56c594bfbf7644f2185fad84/whisper/transcribe.py#L279) for both short-form and long-form audio, while we used to have temperature fallback only for long-form audio.

It aims to solve issues and divergences with the original implementation: 
1. When decoding with timestamps, the Transformers implementation skips the last segment for short-form audio while the OpenAI implementation does not: it will use the sliding window strategy and go to the last generated timestamp (so indeed no differences between inferring short-form and long-form audio). This was not detected in the tests since [test_tiny_timestamp_generation](https://github.com/huggingface/transformers/blob/37ea04013b34b39c01b51aeaacd8d56f2c62a7eb/tests/models/whisper/test_modeling_whisper.py#L2048) and [test_large_timestamp_generation](https://github.com/huggingface/transformers/blob/37ea04013b34b39c01b51aeaacd8d56f2c62a7eb/tests/models/whisper/test_modeling_whisper.py#L2185) are incorrect. 
2. Miscalculation of the `avg_logprobs` that triggered temperature fallback when it should not.
3. When decoding short-form / long-form audio, we returned:
   1. **short-form**`decoder_input_ids + predicted tokens (including the eos token)` 
   2. **long-form** only the `predicted tokens (without eos token)` 
Since short-form and long-form generation are now merged, we need a consistent way of returning outputs (at least for the tokens, we still need to differentiate for past_key_values see here). To be consistent with generate convention and since the tokenizer has the skip_special_tokens argument, I went for option 1.

##  🚨 Important changes 🚨
 ➡️  **Previously:**
	•	Short-form: Returned a `ModelOutput` or `torch.LongTensor`, including decoder input IDs and the EOS token ID.
	•	Long-form: Returned a `Dict` or `torch.LongTensor`, excluding decoder input IDs and the EOS token ID.

➡️  **From now on:**
Short-form and long-form generation are now treated identically, meaning output differentiation based on these modes is no longer applicable.

Decoder input IDs and EOS token IDs are never returned, except in two specific cases: when `return_dict_in_generate=True` and (`return_timestamps=False` or `force_unique_generate_call=True`).

In this case, the output will be a `ModelOutput`, which is the result of the underlying call to GenerationMixin’s generate. Indeed, `return_timestamps=False` ensures no seeking occurs; only a single call to generate is made. Therefore, this output includes both decoder input IDs and the EOS token ID.

## Testing

> [!NOTE]  
> This PR reconciles our implementation with OpenAI's. It should therefore be tested with the updated and verified tests introduced in #34111

## Evaluations

Let’s verify the effectiveness of this fix. We will evaluate both accuracy and inference speed. To do this, we will test on four short-form test sets and two long-form test sets (effectively filtering samples as ≤30 sec and >30 sec). For the short-form sets, we will compare results with the current main branch of Transformers on the first 100 samples. (Indeed, Issue #2, which involves miscalculation of the avg_logprobs, causes the full test set to be too slow.)

⚠️ Moreover, it was also tested with [this PR](https://github.com/huggingface/transformers/pull/34537) merged. 

As explained in [this PR](https://github.com/huggingface/transformers/pull/34111), we need to use a simple Whisper fork to ensure consistency with the same input features.

> [!IMPORTANT]
> **TL;DR:** We achieve perfect 1-to-1 matching in prediction results with greedy decoding for both short-form and long-form samples, validating that our implementation matches OpenAI’s original decoding algorithm. For short-form, this PR is ~5x faster than the current implementation (due to multiple incorrect temperature fallbacks).❗

<details>
  <summary>Results 📊</summary>
  
### → short-form (first 100 samples)

Set 1: [edinburghcstr/ami](https://huggingface.co/datasets/edinburghcstr/ami), config `"ihm"`, split `test[:100]`
Set 2: [distil-whisper/chime4](https://huggingface.co/datasets/distil-whisper/chime4), config `"1-channel"`, split `test[:100]`
Set 3: [google/fleurs](https://huggingface.co/datasets/google/fleurs), config `"en_us"`, split `test[:100]`

Wandb results [here](https://wandb.ai/eustache-lebihan-hf/benchmark-whisper-short)!

|                                       | set 1 - WER | set 1 - RTFx | set 1 - WER | set 1 - RTFx | set 1 - WER | set 1 - RTFx |
| ------------------------------------- | ----------- | ------------ | ----------- | ------------ | ----------- | ------------ |
| **whisper-orig ([script](https://gist.github.com/eustlb/1828634ea898b77e47ebdaa38268cf83/2eb34add5400aa376e596b1cb8e576ea09c52cd2))**      | 17.35       | 6.55         | 4.62        | 12.21        | 3.78        | 14.61        |
| **whisper-orig-fork ([script](https://gist.github.com/eustlb/465cbe9319627089ab127b404012b9f4/44260c3aa0017f0d3776f9259322298d096c7dc0))** | 19.4        | 6.45         | 4.55        | 12           | 3.78        | 14.12        |
| **this PR ([script](https://gist.github.com/eustlb/1fa238d17d4f1f49b19b79c834fb5c7d/c31de96fd7e9d6388219c2cb621063352b2fcea6))**           | 19.4        | 3.49         | 4.55        | 8.05         | 3.78        | 10.59        |
| **currrent main ([script](https://gist.github.com/eustlb/1fa238d17d4f1f49b19b79c834fb5c7d/ed5d5d18c8e97eb8416631506a448ce4e50b8fa1))**     | 25.8        | 0.73         | 5.67        | 1.49         | 7.09        | 1.9          |</details>

### → short-form (full test sets)

Set 1: [edinburghcstr/ami](https://huggingface.co/datasets/edinburghcstr/ami), config `"ihm"`, split `test`
Set 2: [distil-whisper/chime4](https://huggingface.co/datasets/distil-whisper/chime4), config `"1-channel"`, split `test`
Set 3: [google/fleurs](https://huggingface.co/datasets/google/fleurs), config `"en_us"`, split `test`

Wandb results [here](https://wandb.ai/eustache-lebihan-hf/benchmark-whisper-short)!

|                       | **Set 1 - WER** | **Set 1 - RTFx** | **Set 2 - WER** | **Set 2 - RTFx** | **Set 3 - WER** | **Set 3 - RTFx** |
| --------------------- | --------------- | ---------------- | --------------- | ---------------- | --------------- | ---------------- |
| **whisper-orig ([script](https://gist.github.com/eustlb/1828634ea898b77e47ebdaa38268cf83/185f8c8e8e90e087c548232dad43e1010a3c5d31))**      | 16.22           | 7.86             | 10.74           | 11.45            | 4.21            | 13.79            |
| **whisper-orig-fork ([script](https://gist.github.com/eustlb/465cbe9319627089ab127b404012b9f4/b21c03011b7d8bcf9af7f937bfa35439608fb052))** | 16.36           | 7.91             | 11.08           | 11.51            | 4.14            | 13.90            |
| **this PR ([script](https://gist.github.com/eustlb/1fa238d17d4f1f49b19b79c834fb5c7d/819ec3d768df5d2e41b9367b7cc3301330476d02))**           | 16.36           | 4.18             | 11.08           | 7.51             | 4.14            | 10.67            |

### → long-form 

Set 1: [distil-whisper/tedlium-long-form](https://huggingface.co/datasets/distil-whisper/tedlium-long-form), config `"default"`, split `test`
Set 2: [distil-whisper/meanwhile](https://huggingface.co/datasets/distil-whisper/meanwhile), config `"default"`, split `test`

Wandb results [here](https://wandb.ai/eustache-lebihan-hf/benchmark-whisper-long)!

|                                       | **Set 1 - WER** | **Set 1 - RTFx** | **Set 2 - WER** | **Set 2 - RTFx** |
| ------------------------------------- | --------------- | ---------------- | --------------- | ---------------- |
| **whisper-orig ([script](https://gist.github.com/eustlb/1828634ea898b77e47ebdaa38268cf83/4619d515e03708464050aa0f0b205536ea6497db))**      | 172.15          | 4.98             | 264.66          | 4.09             |
| **whisper-orig-fork ([script](https://gist.github.com/eustlb/465cbe9319627089ab127b404012b9f4/8c717feb0c9ea98e0849945c78b62c381ce94069))** | 172.15          | 4.83             | 264.66          | 4.00             |
| **this PR ([script](https://gist.github.com/eustlb/1fa238d17d4f1f49b19b79c834fb5c7d/52cba2a9e8109f022418b9874b5791717abb4284))**           | 172.15          | 4.88             | 264.66          | 3.98             |
| **current main ([script](https://gist.github.com/eustlb/1fa238d17d4f1f49b19b79c834fb5c7d/9a343e35d28ea39d7dac8f39d5faffafbb4e613f))**     | 172.15          | 4.92             | 264.66          | 3.98             |


Comments:
Even if we usually return the `decoder_input_ids` and `eos_token` with `generate` in Transformers, and this is why we have a `skip_special_tokens=True` option in the tokenizer, I think it is better here **not to return** context tokens. Indeed, this would imply a lot of ambiguities: since we actually have multiple calls to generate when doing sequential decoding, why would we include those tokens only for the first segment of the concatenated sequence of tokens and the last (for the eos token)? This would let users think that generation was indeed in one shot and that tokens are indeed the concatenated ones. Likewise, it would be even worse for the returned segments: some will have the context tokens, and some won't, depending on if the segment is the first of a new call to generate.


 For these reasons, I think it is better to stick with OpenAI's choice: return only the generated tokens. Moreover, this is the way it is currently implemented in Transformers : long-form generation does not return context tokens. As a consequence, this also comes with the advantage of requiring fewer changes in the current codebase, reducing the potential for mistakes. WDYT @ylacombe? Also pinging @ArthurZucker here since you've worked on Whisper integration. 

**Correction:**
After discussion, it has been decided to rather go for solution 2: return only the generated tokens and skip all the special tokens.
Pros: fewer changes, no ambiguities. 
Cons: need to overwrite generic tests of GenerationMixin

Sounds good!

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_34135). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Let's make sure the last failing tests are fixed before asking for @ArthurZucker's review!

Thanks a lot @ylacombe for the review. I updated it based on your comments (see the updated PR comment) !  

The four failing slow tests are expected! Merging 🤗