Mismatch of implementations of attention mask in transformers and tokenizers 
Author: superRookie007
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.10.2


Models: BERT and other Transformer models

Library:
- tokenizers: @LysandreJik

## Problem and solution:
<!-- Problems and solution -->
In the current implementation of transformers, the attention mask is added to the attention scores before softmax. For instance: https://github.com/huggingface/transformers/blob/3ab0185b061baae207efed02799dd424ee8377f1/src/transformers/models/bert/modeling_bert.py#L326

When this kind of additive mask is used before softmax, then the masking values should be -inf. However, the tokenizers of transformers library outputs 0 for the attention masks (for paddings) and 1 for the other elements: https://huggingface.co/transformers/glossary.html#attention-mask
This would have very unpredictable behaviour on the final attention values after softmax. Sometimes the attention mask may have no effect at all depending on the magnitude of the attention scores before softmax. 

An alternative implementation is using multiplicative or logical_and to apply attention mask to attention scores (Pytorch, Tensorflow, Jax). Then it is fine to assign value 0 to paddings in the attention mask. 




Comments:
Note that the head mask is implemented using multiplication: https://github.com/huggingface/transformers/blob/3ab0185b061baae207efed02799dd424ee8377f1/src/transformers/models/bert/modeling_bert.py#L337

Hi,

the `attention_mask` is first preprocessed by a function called `get_extended_attention_mask` as can be seen [here](https://github.com/huggingface/transformers/blob/51e5eca612d24896165ba8f7c83ecd0e8f695aa4/src/transformers/models/bert/modeling_bert.py#L963). This function is defined in `modeling_utils.py`. This function turns the `attention_mask` into a tensor that is 0 for positions we want to attend to, and -10.000 for positions we don't want to attend to as seen [here](https://github.com/huggingface/transformers/blob/51e5eca612d24896165ba8f7c83ecd0e8f695aa4/src/transformers/modeling_utils.py#L294-L301). 

@NielsRogge Ah, I see. That makes sense. Thanks for the clarification.

However, are there any reasons why it was implemented this way? Because at least on the surface, it makes the code slightly more complicated, and more confusing and difficult to understand. The multiplicative head mask just a few lines after makes it even more confusing to people who are not fully familiar with the code base. And is -10000 enough to guarantee that the behaviour is always predictable (I am not sure, maybe it is)?  

If there are specific reasons why it has been implemented this way, it might be a good idea to add a few comments at https://github.com/huggingface/transformers/blob/3ab0185b061baae207efed02799dd424ee8377f1/src/transformers/models/bert/modeling_bert.py#L326 and https://github.com/huggingface/transformers/blob/51e5eca612d24896165ba8f7c83ecd0e8f695aa4/src/transformers/models/bert/modeling_bert.py#L963
so that people don't have to dig deep into the code to find out what is going on.

Just some suggestions, otherwise this issue can be considered closed.


> However, are there any reasons why it was implemented this way?

This was because the original BERT authors implemented it that way.

> It might be a good idea to add a few comments

Fair point! cc @LysandreJik 


Indeed, adding a few comments would be helpful! Do you want to try your hand at it?

@LysandreJik Did you mean me or @NielsRogge ?
If it is not urgent, I can do it sometime next week.

Glad to hear it @superRookie007, looking forward to it!

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/master/CONTRIBUTING.md) are likely to be ignored.