Unable to ignore pad tokens when using `decoder_input_ids` and `decoder_attention_mask` with `BartForConditionalGeneration`
Author: pullelys
Pull request: no

## Environment info
<!-- You can run the command `transformers-cli env` and copy-and-paste its output below.
     Don't forget to fill out the missing fields in that output! -->

- `transformers` version: 4.16.2
- Platform: Linux-3.10.0-1160.45.1.el7.x86_64-x86_64-with-glibc2.2.5
- Python version: 3.8.5
- PyTorch version (GPU?): 1.10.2+cu102 (True)
- Tensorflow version (GPU?): not installed (NA)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using GPU in script?: Yes, but it's not necessary
- Using distributed or parallel set-up in script?: No

### Who can help
<!-- Your issue will be replied to more quickly if you can figure out the right person to tag with @
 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- ALBERT, BERT, XLM, DeBERTa, DeBERTa-v2, ELECTRA, MobileBert, SqueezeBert: @LysandreJik
- T5, BART, Marian, Pegasus, EncoderDecoder: @patrickvonplaten
- Blenderbot, MBART: @patil-suraj
- Longformer, Reformer, TransfoXL, XLNet, FNet, BigBird: @patrickvonplaten
- FSMT: @stas00
- Funnel: @sgugger
- GPT-2, GPT: @patrickvonplaten, @LysandreJik
- RAG, DPR: @patrickvonplaten, @lhoestq
- TensorFlow: @Rocketknight1
- JAX/Flax: @patil-suraj
- TAPAS, LayoutLM, LayoutLMv2, LUKE, ViT, BEiT, DEiT, DETR, CANINE: @NielsRogge
- GPT-Neo, GPT-J, CLIP: @patil-suraj
- Wav2Vec2, HuBERT, SpeechEncoderDecoder, UniSpeech, UniSpeechSAT, SEW, SEW-D, Speech2Text: @patrickvonplaten, @anton-l

If the model isn't in the list, ping @LysandreJik who will redirect you to the correct contributor.

Library:

- Benchmarks: @patrickvonplaten
- Deepspeed: @stas00
- Ray/raytune: @richardliaw, @amogkam
- Text generation: @patrickvonplaten @narsil
- Tokenizers: @SaulLu
- Trainer: @sgugger
- Pipelines: @Narsil
- Speech: @patrickvonplaten, @anton-l
- Vision: @NielsRogge, @sgugger

Documentation: @sgugger

Model hub:

- for issues with a model, report at https://discuss.huggingface.co/ and tag the model's creator.

HF projects:

- datasets: [different repo](https://github.com/huggingface/datasets)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Examples:

- maintained examples (not research project or legacy): @sgugger, @patil-suraj

For research projetcs, please ping the contributor directly. For example, on the following projects:

- research_projects/bert-loses-patience: @JetRunner
- research_projects/distillation: @VictorSanh

 -->
@patrickvonplaten
## Information

Model I am using (Bert, XLNet ...): `BartForConditionalGeneration`

The problem arises when using:
* [ ] the official example scripts: (give details below)
* [x] my own modified scripts: (give details below)

Script: https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/3fb1b593f234052be264cb531c0c56502277770e#file-mask_filling-py
Output: https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/3fb1b593f234052be264cb531c0c56502277770e#file-output-txt

The tasks I am working on is:
* [ ] an official GLUE/SQUaD task: (give the name)
* [x] my own task or dataset: (give details below)

Mask filling as in the official code snippet: https://huggingface.co/docs/transformers/model_doc/bart#mask-filling

My goal is very simple: I want to generate the exact same output as the official code snippet when manually specifying `decoder_input_ids` where `decoder_input_ids` contains some pad tokens, i.e. pad tokens should be ignored. How can I achieve this goal? 

Sidenote: My actual task is not to replicate the default output when using pad tokens in `decoder_input_ids` just for the sake of using pad tokens but requires resolving this issue.

First, I generate the output without manually specifying `decoder_input_ids` using `model.generate(**kwargs)`. Per default, `use_cache=True` is used in order to  speed up decoding. However, when `decoder_input_ids` is manually specified, then `use_cache=False` is required. I use `use_cache=False` for all generations.

For more information, read the explanation in `BART_INPUTS_DOCSTRING`: 
https://github.com/huggingface/transformers/blob/db7d6a80e82d66127b2a44b6e3382969fdc8b207/src/transformers/models/bart/modeling_bart.py#L590-L603
https://github.com/huggingface/transformers/blob/db7d6a80e82d66127b2a44b6e3382969fdc8b207/src/transformers/models/bart/modeling_bart.py#L634-L648

For all outputs, I also print the beam search score of the generated sequence which allows me to check if two outputs have the exact same score.

Default output:
```
DEFAULT OUTPUT WITHOUT MANUAL DECODER INPUT 

-0.4835893213748932


UN Chief Says There Is No Plan to Stop Chemical Weapons in Syria

```
Outputs with manually specified `decoder_input_ids`:
```
decoder_input_ids:  [[2]]

-0.4835893213748932


UN Chief Says There Is No Plan to Stop Chemical Weapons in Syria

----------------------------------------------------------------------------------------------------
decoder_input_ids:  [[2 0]]

-0.4835893213748932


UN Chief Says There Is No Plan to Stop Chemical Weapons in Syria

```
As expected, when `decoder_input_ids=[[2]]` or `decoder_input_ids=[[2,0]]` the default output is exactly replicated, given that when `decoder_input_ids` is not manually specified `BartForConditionalGeneration` always starts `decoder_input_ids` with `model.config.decoder_start_token_id=2` and then forces `model.config.bos_token_id=0` to be generated due to `BartForConditionalGeneration.from_pretrained("facebook/bart-large", forced_bos_token_id=0)`.

Outputs when `decoder_input_ids` contains pad tokens where `model.config.pad_token_id=1`:
```
decoder_input_ids:  [[2 1 0]]

-0.852648913860321

<pad>
UN Chief Says

----------------------------------------------------------------------------------------------------
decoder_input_ids:  [[1 2 0]]

-0.7636396884918213
<pad>

 UN

----------------------------------------------------------------------------------------------------
decoder_input_ids:  [[2 0 1]]

-0.5718380212783813


<pad>.UN Chief Says There Is No Place for Human Rights in Syria

----------------------------------------------------------------------------------------------------
decoder_input_ids:  [[2 0 1 1]]

-0.6295203566551208


<pad><pad> in Syria.UN Chief Says There Is No Alternative to Assad

----------------------------------------------------------------------------------------------------
decoder_input_ids:  [[2 0 1 1 1]]

-0.49761223793029785


<pad><pad><pad>.UN Chief Says There Is No Alternative to Assad in Syria

```
I wasn't sure where exactly to place the pad tokens, so I tried all three possibilities: left, center, right. Placing the pad token to the left and in the center yields short outputs and placing the pad token to the right yields the best although not the same output as for the default and with a strange dot in the beginning. Having multiple pad tokens also changes the output.

According to the docstring in `BART_INPUTS_DOCSTRING`, this behaviour is not expected and pad tokens should be ignored by default without having to manually specify `decoder_attention_mask`:
https://github.com/huggingface/transformers/blob/db7d6a80e82d66127b2a44b6e3382969fdc8b207/src/transformers/models/bart/modeling_bart.py#L604-L610

Given that pad tokens are not ignored by default, I manually specify `decoder_attention_mask` such that pad tokens are ignored. However, by default `decoder_attention_mask` is not used when using `model.generate(**kwargs)`. Hence, I create the class `CustomBart(BartForConditionalGeneration, GenerationMixin)` where I exactly copy the two functions below and add some code to enable `decoder_attention_mask` to be passed along when using it in `model.generate(**kwargs)`. The adjustments are clearly marked. Note that I use `CustomBart` for all generations since whenever `decoder_attention_mask` is not specified, it generates the exact same output as `BartForConditionalGeneration`.

https://github.com/huggingface/transformers/blob/0fe17f375a4f0fdd9aea260d0645ccfd4896e958/src/transformers/models/bart/modeling_bart.py#L1366

Adjustment: https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/3fb1b593f234052be264cb531c0c56502277770e#file-mask_filling-py-L32-L36

https://github.com/huggingface/transformers/blob/0fe17f375a4f0fdd9aea260d0645ccfd4896e958/src/transformers/generation_utils.py#L559

Adjustment: https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/3fb1b593f234052be264cb531c0c56502277770e#file-mask_filling-py-L72-L78

Output when manually specifying `decoder_attention_mask` such that it ignores pad tokens in `decoder_input_ids`:

```
decoder_input_ids:  [[2 0 1]]
decoder_attention_mask:  [[1 1 0]] 

Traceback (most recent call last):
  File "mask_filling.py", line 179, in <module>
    print_results(kwargs)
  File "mask_filling.py", line 93, in print_results
    outputs = model.generate(**kwargs)
  File "venv/lib64/python3.8/site-packages/torch/autograd/grad_mode.py", line 28, in decorate_context
    return func(*args, **kwargs)
  File "transformers/src/transformers/generation_utils.py", line 1234, in generate
    return self.beam_search(
  File "transformers/src/transformers/generation_utils.py", line 1974, in beam_search
    outputs = self(
  File "venv/lib64/python3.8/site-packages/torch/nn/modules/module.py", line 1102, in _call_impl
    return forward_call(*input, **kwargs)
  File "transformers/src/transformers/models/bart/modeling_bart.py", line 1326, in forward
    outputs = self.model(
  File "venv/lib64/python3.8/site-packages/torch/nn/modules/module.py", line 1102, in _call_impl
    return forward_call(*input, **kwargs)
  File "transformers/src/transformers/models/bart/modeling_bart.py", line 1216, in forward
    decoder_outputs = self.decoder(
  File "venv/lib64/python3.8/site-packages/torch/nn/modules/module.py", line 1102, in _call_impl
    return forward_call(*input, **kwargs)
  File "transformers/src/transformers/models/bart/modeling_bart.py", line 1007, in forward
    attention_mask = self._prepare_decoder_attention_mask(
  File "transformers/src/transformers/models/bart/modeling_bart.py", line 898, in _prepare_decoder_attention_mask
    expanded_attn_mask if combined_attention_mask is None else expanded_attn_mask + combined_attention_mask
RuntimeError: The size of tensor a (3) must match the size of tensor b (4) at non-singleton dimension 3
```
Both `decoder_inputs_ids` and `decoder_attention_mask` should have shape `(batch_size, target_sequence_length)` according to `BART_INPUTS_DOCSTRING` as can be seen above. However, this produces a `RuntimeError`.

## To reproduce

Steps to reproduce the behavior:

1. Run my script using `transformers` version 4.16.2: https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/3fb1b593f234052be264cb531c0c56502277770e#file-mask_filling-py
2. Compare your output with my output: https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/3fb1b593f234052be264cb531c0c56502277770e#file-output-txt

<!-- If you have code snippets, error messages, stack traces please provide them here as well.
     Important! Use code tags to correctly format your code. See https://help.github.com/en/github/writing-on-github/creating-and-highlighting-code-blocks#syntax-highlighting
     Do not use screenshots, as they are hard to read and (more importantly) don't allow others to copy-and-paste your code.-->

## Expected behavior

<!-- A clear and concise description of what you would expect to happen. -->

I expect one of the following, ideally both:

1. Pad tokens should be ignored by default when manually specifying `decoder_input_ids`.
2. One should be able to use `decoder_attention_mask` without getting a `RuntimeError`.

As a consequence, the generated output should be the same as the default when manually specifying `decoder_input_ids` with additional pad tokens or if necessary, additionally specifying `decoder_attention_mask` to ignore pad tokens.

Comments:
This post in the Hugging Face forum is related to my issue, albeit for another model:
https://discuss.huggingface.co/t/is-t5-expected-to-ignore-padding-tokens-in-decoder-input-ids-when-decoder-attention-mask-is-not-provided/10271

I've identified the issue. During beam search, there is a while loop which decodes the next tokens one step at a time. At each step `input_ids` (these are the `decoder_input_ids`) get extended with the next tokens found via beam search. However, `transformers` version 4.16.2 does not extend the `decoder_attention_mask` after each decoding step when it's manually specified in `model_kwargs` in `beam_search`:

https://github.com/huggingface/transformers/blob/db7d6a80e82d66127b2a44b6e3382969fdc8b207/src/transformers/generation_utils.py#L1794-L1809

Hence the `RuntimeError` described above appears. 

More specifically, here the `input_ids` get extendend:
https://github.com/huggingface/transformers/blob/db7d6a80e82d66127b2a44b6e3382969fdc8b207/src/transformers/generation_utils.py#L2039

In the next loop, the error occurs because `decoder_attention_mask` has not been extended:

https://github.com/huggingface/transformers/blob/db7d6a80e82d66127b2a44b6e3382969fdc8b207/src/transformers/generation_utils.py#L1972-L1979

I could fix the issue, by adjusting the function `prepare_inputs_for_generation` such that at each step `decoder_attention_mask` gets extended if needed:

https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/30fe77c0651ac949ee61be7f46113262bbecce45#file-mask_filling-py-L11-L58

Even though I can now use `decoder_attention_mask` to ignore pad tokens, surprisingly, depending on where the pad token is placed (left, center, right), a different output gets generated with the best result (according to the beam score) when the pad token is placed in the center, i.e. after the decoder start token and before the BOS token. However, the default output could still not be exactly replicated. Moreover, the output also changes with the padding size which should not be the case in my opinion.

```
DEFAULT OUTPUT WITHOUT MANUAL DECODER INPUT 

-0.4835893213748932


UN Chief Says There Is No Plan to Stop Chemical Weapons in Syria

----------------------------------------------------------------------------------------------------
decoder_input_ids:  [[2 0 1]]
decoder_attention_mask:  [[1 1 0]] 

-0.5901848673820496


<pad>.UN Chief Says There Is No Plan to Stop ISIS in Syria

----------------------------------------------------------------------------------------------------
decoder_input_ids:  [[2 1 0]]
decoder_attention_mask:  [[1 0 1]] 

-0.36968478560447693

<pad>
UN Chief Says There Is No Alternative to Assad in Syria

----------------------------------------------------------------------------------------------------
decoder_input_ids:  [[2 1 1 1 0]]
decoder_attention_mask:  [[1 0 0 0 1]] 

-0.3912160396575928

<pad><pad><pad>
UN Chief Says There Is No Plan B for Peace in Syria

----------------------------------------------------------------------------------------------------
decoder_input_ids:  [[1 2 0]]
decoder_attention_mask:  [[0 1 1]] 

-1.161877155303955
<pad>

''',,',''"'""''

```

Instead of updating `decoder_attention_mask` in the function `prepare_inputs_for_generation`, it should be done in the function `_update_model_kwargs_for_generation`:
https://github.com/huggingface/transformers/blob/db7d6a80e82d66127b2a44b6e3382969fdc8b207/src/transformers/generation_utils.py#L2041-L2043

Here is the adjusted `_update_model_kwargs_for_generation` function:

https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/1163b4affe94df16bc088c7f8f16ff900bcd82a7#file-mask_filling-py-L75-L84



Hey @pullelys,

Sorry I'm a bit lost here. Could you maybe copy-paste a reproducible code snippet here that we can run in a Python shell to spot the error? :-)

Sure, here are the steps to reproduce the behavior:

1. Run my script using `transformers` version 4.16.2: https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/3fb1b593f234052be264cb531c0c56502277770e#file-mask_filling-py
2. Compare your output with my output: https://gist.github.com/pullelys/43ebb9d2785d858d17b4e2e61464dc48/3fb1b593f234052be264cb531c0c56502277770e#file-output-txt

Hey @pullelys, 

Sorry it'll take me too much time to dive into this. This is a very long script and includes a custom model definition. We cannot guarantee that custom models built on top of Transformers work as expected.

Is there also a problem with the official `BartForConditionalGeneration(...)` model? If yes, could you try to put a **minimal**, **reproducible** code-snippet below and explain what exactly the error is? This would help us a great deal to better understand the issue here. 

Thanks a lot!



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.