Mistake in the BART doc & inconsistency between code & doc
Author: JulesGM
Pull request: no
Labels: name: bug

Version: Most recent in the Github repo.
Model: BART.

Description:
https://github.com/huggingface/transformers/blob/main/src/transformers/models/bart/modeling_bart.py#L627 says to look at `modeling_bart._prepare_decoder_inputs` to modify the default behavior of the way `decoder_attention_mask` behaves by default, but there is no `_prepare_decoder_inputs` anywhere in the Huggingface Transformers repository. I guess it's an artifact from a previous version, and that the function is now called `_prepare_decoder_attention_mask`. However, this method doesn't seem to look at the values of the decoder inputs anywhere, so I don't think it does what the doc says, ie mask the pad tokens. Or is this done somewhere else?

Thanks.


### Who can help?

@patil-suraj

### Expected behavior

```shell
Mask pad tokens by default in the decoder.
```


Comments:
One could fix this by putting
```python
if attention_mask is None:
            attention_mask = input_ids != self.config.pad_token_id
```
somewhere

Hi @JulesGM ! Thanks for reporting this, the doc should be changed to mentioned the `_prepare_decoder_attention_mask`  method. But note that, the `_prepare_decoder_attention_mask` prepares the casual mask for the decoder and then combines it with the `decoder_attention_mask` if it's passed by the user.  The causal mask is not meant to ignore padding tokens hence it doesn't look at the `decoder_input_ids`.

Also, we don't automatically prepare `decoder_attention_mask` because we can't always assume that the user wants to mask padding tokens. So the user should pass it, if he/she wants it.

Great thank. I mentioned that because the doc says it did. Just FYI, other
models also have references to the function that doesn't exist.
I wish there was an easy way to ignore pad tokens in decoder input, to be
able to condition on some already generated text in the decoder,
like scratchpads https://arxiv.org/pdf/2112.00114.pdf. Right now it's very
hard, I have to write a custom way to cache the previous positions to only
increment the positional encoders the proper amount with caching.


On Thu, May 12, 2022 at 9:05 AM Suraj Patil ***@***.***>
wrote:

> Hi @JulesGM <https://github.com/JulesGM> ! Thanks for reporting this, the
> doc should be changed to mentioned the _prepare_decoder_attention_mask
> method. But note that, the _prepare_decoder_attention_mask prepares the
> casual mask for the decoder and then combines it with the
> decoder_attention_mask if it's passed by the user. The causal mask is not
> meant to ignore padding tokens hence it doesn't look at the
> decoder_input_ids.
>
> Also, we don't automatically prepare decoder_attention_mask because we
> can't always assume that the user wants to mask padding tokens. So the user
> should pass it, if he/she wants it.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/huggingface/transformers/issues/17191#issuecomment-1124969687>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAYU34MU4AOV42UGDXLCSR3VJT625ANCNFSM5VWKKPAQ>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>