Whisper doesn't compute positional embeddings properly when given batches of prompt tokens
Author: andyehrenberg
Pull request: no

### System Info

v4.25.1 on M1 Mac with python 3.8

### Who can help?

@sanchit-gandhi @patrickvonplaten @anton-l 

### Information

- [ ] The official example scripts
- [ ] 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

When we want to run Whisper generation for a batch of samples with different prompt lengths (prefix tokens given to the decoder), positional embeddings for the decoder are improperly computed. It assumes all sequences have the same `past_key_values_length`, but this is not true in general.

Scenario:
`decoder_input_ids = [50361, 45431,  2584, 28682, 13, 50258, 50257, 50257]`
(`"
Something completely irrelevant.


"`)

`model.generate(input_features, decoder_input_ids=decoder_input_ids, decoder_attention_mask=decoder_attention_mask)` will not give the correct output because at the beginning of decoding, the pad tokens won't be taken into account that the positional embedding will be off.

### Expected behavior

Instead of tracking `past_key_values_length`, it should use the attention mask to compute position ids. The current implementation is more based off of encoder-decoder architectures that would never do decoder prompting, but it should take more inspiration from decoder-only models to handle prompting. This is done for the Flax implementation in #20479 

Comments:
cc @ArthurZucker 

Thanks for opening this good issue 🤗 I'll have a proper look, I think you insight is pretty good. 

I have similar issue while using whisper with `padding=True` and I got this error:

```
RuntimeError: The size of tensor a (359) must match the size of tensor b (1500) at non-singleton dimension 1
```

However, there isn't any issue if I use `padding=max_length`.

The padding in whisper should always be set to `max_length` , and you should not really modify it. We should probably prevent people from using just `True`. 

@hannan72's issue is separate to what I'm describing. But yes, padding should always be `max_length` - the issue I'm describing arises as a result of pad tokens being added to shorter sequences in batches (and won't raise any errors - it's just that Whisper's handling of multiple sequence lengths under the hood is flawed and would be fixed by computing `position_ids` based off `attention_mask`).

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.

Any update on this issue?

@samuelazran See https://github.com/huggingface/transformers/pull/21455 - feel free to run with that and give any fixes. My Flax PR also shows how to handle this.

> @samuelazran See #21455 - feel free to run with that and give any fixes. My Flax PR also shows how to handle this.

Thank you! I will test it.
Could you provide a code example of using prompts for training / inference?
I have an implementation but not sure yet:
https://discuss.huggingface.co/t/adding-prompt-context-to-whisper-with-huggingface-transformers/31070

just like @samuelazran, would really like to see the example and the #21455 in, being able to use 🤗 transformers directly is very helpful compared to using the external (original) library.

Prompting is an ongoing PR here: https://github.com/huggingface/transformers/pull/22496

Regarding #21455 - I think this should be handled by the aforementioned PR