Whisper tokenizer decode function ignores timestamp tokens after v4.34.0 (the big refactor)
Author: versae
Pull request: no

### System Info

- `transformers` version: 4.34.0
- Platform: Linux-5.19.0-40-generic-x86_64-with-glibc2.35
- Python version: 3.10.6
- Huggingface_hub version: 0.17.3
- Safetensors version: 0.3.1
- Accelerate version: 0.20.3
- Accelerate config:    not found
- PyTorch version (GPU?): 2.0.0+cu117 (True)
- Tensorflow version (GPU?): 2.12.0 (False)
- Flax version (CPU?/GPU?/TPU?): 0.6.11 (cpu)
- Jax version: 0.4.12
- JaxLib version: 0.4.12
- Using GPU in script?: no
- Using distributed or parallel set-up in script?: no



### Who can help?

@sanchit-gandhi, @peregilk 

### Information

- [X] The official example scripts
- [X] My own modified scripts

### Tasks

- [X] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [X] My own task or dataset (give details below)

### Reproduction

Install `transformers>=4.34.0` and run this code:

```python
from transformers import AutoTokenizer, AddedToken

tokenizer = AutoTokenizer.from_pretrained("openai/whisper-large-v2")
tokenizer.set_prefix_tokens(language="en", task="transcribe", predict_timestamps=True)

print(
    tokenizer.encode("<|0.00|>", add_special_tokens=False),
    tokenizer.decode(tokenizer.encode("<|0.00|>"))
)
```

The output will be

```bash
[50364] 




```

Which ignores the timestamp token when decoding.

### Expected behavior

With versions of `transformers<4.34.0`, the timestamp tokens will be correctly decoded. The same code will produce:

```bash
[50364] 


<|0.00|>

```

Comments:
Well, based on #26054, it seems a new undocumented parameter `decode_with_timestamps` has to be passed in:

```python
print(
    tokenizer.encode("<|0.00|>", add_special_tokens=False),
    tokenizer.decode(tokenizer.encode("<|0.00|>"), decode_with_timestamps=True)
)
```
```bash
[50364] 


<|0.00|>

```

It seems to me that if the timestamp tokens are there and the user sets `predict_timestamps=True`, `decode_with_timestamps` should default to `True` or maybe not even exist as a parameter?


I think we have some of this for BC but also performance reasons, cc @sanchit-gandhi 

That would be strange. With the previous behavior, and before the OpenAI Whisper models included the timestamp tokens, if you added them yourself using the `.add_tokens()` method, by default those tokens would be returned when decoding as well.

Hey @versae! Thanks for flagging this - this is a bit of a tricky edge case since the HF tokenizers were only _recently_ updated to have the timestamp tokens in their vocabulary. Prior to this, we followed the _original_ OpenAI behaviour, where the timestamps were not part of the vocabulary.

If the timestamps were added by the user to the tokenizer (i.e. custom behaviour), then the tokenizer would **always** return predicted timestamps as part of the transcriptions. This is because we didn't design the tokenizer for this custom use-case, but assumed users would be using the default vocabulary. This then gave incorrect behaviour,  as the tokenizer returned the timestamp tokens even if `decode_with_timestamps=False`.

This is in contradiction to the prior default behaviour: when timestamp tokens wer **not** part of the vocabulary, then the user needs to set `decode_with_timestamps=True` to decode the timestamp tokens to text. If they do not, then the timestamp tokens will not be decoded, but rather filtered and removed from the transcription.

The current behaviour on `main` fixed the custom tokenizer behaviour, which was inconsistent with the previous one: now that timestamp tokens are part of the default tokenizer vocabulary, we have updated the tokenizer to only return timestamps when `decode_with_timestamps=True`, in-keeping with how the tokenizer operated before the timestamp tokens were added

The fact that the custom tokenizer returned the timestamp tokens with `decode_with_timestamps=False` before was a bit of a bug, but not a case we could design for since it was custom behaviour expanding the vocabulary of the tokenizer

Hope that explains things @versae! Let me know if you have any questions

Thanks for the explanation, @sanchit-gandhi! I still think that if the user sets `predict_timestamps=True` is because is expecting to see timestamp tokens in the output, which would make `decode_with_timestamps` default to `True`.

Indeed this is how we set it in the `pipeline`: one input argument `return_timestamps` triggers all of the timestamp args under the hood. However, for the `model` + `processor` API it needs to be set twice, each time by the user. This is because there's no coupling between the `model` and the `processor`, and so we can't propagate the arguments forward automatically.

> Well, based on #26054, it seems a new undocumented parameter `decode_with_timestamps` has to be passed in:
> 
> ```python
> print(
>     tokenizer.encode("<|0.00|>", add_special_tokens=False),
>     tokenizer.decode(tokenizer.encode("<|0.00|>"), decode_with_timestamps=True)
> )
> ```
> 
> ```shell
> [50364] 


<|0.00|>

> ```
> 
> It seems to me that if the timestamp tokens are there and the user sets `predict_timestamps=True`, `decode_with_timestamps` should default to `True` or maybe not even exist as a parameter?

@versae my solution was the following

```python
predicted_ids = model.generate(input_features,
                                            max_length=model.config.max_target_positions,
                                            num_beams=5,
                                            length_penalty=1.0,
                                            repetition_penalty=1.2,
                                            do_sample=do_sample,
                                            temperature=temperature,
                                            return_dict_in_generate=True,
                                            output_scores=True,
                                            output_hidden_states=True,
                                            output_attentions=True,
                                            early_stopping=early_stopping)
            
                # Decode the outputs, including the timestamps
                decoded_transcription = processor.batch_decode(predicted_ids['sequences'], skip_special_tokens=False)[0]
                decoded_tokens = processor.tokenizer.tokenize(decoded_transcription)
                
                # Extract timestamps
                transcription_chunk = []
                current_text = []
                current_start_time = None
                for token in decoded_tokens:
                    if token.startswith("<|"):
                        if token not in {"
", "
", "
", "
", "
"}:
                            if current_text:
                                end_time = start_idx / sampling_rate + processor.tokenizer.convert_tokens_to_ids([token])[0] * HOP_LENGTH / sampling_rate
                                transcription_chunk.append({
                                    "start_time": current_start_time,
                                    "end_time": end_time,
                                    "text": " ".join(current_text)
                                })
                                current_text = []
                            current_start_time = start_idx / sampling_rate + processor.tokenizer.convert_tokens_to_ids([token])[0] * HOP_LENGTH / sampling_rate
                    else:
                        if not current_text:
                            current_start_time = start_idx / sampling_rate
                        current_text.append(token.replace("Ġ", " "))  # Clean up token format

                if current_text:
                    end_time = len(audio_chunk) / sampling_rate
                    transcription_chunk.append({
                        "start_time": current_start_time,
                        "end_time": end_time,
                        "text": " ".join(current_text)
                    })
```

but the timestamps obtained are apparently the same for different audio!