Fix -1e4 as attn mask
Author: ydshieh
Pull request: yes

# What does this PR do?

Fix the issues regarding `-1e4` as attention mask.

Fix #17215 #17121 #14859

Comments:
_The documentation is not available anymore as the PR was closed or merged._

Generally, this looks good to me. I'd prefer though to not factor out a one-liner into a function (even if we have to add the one-liner 100+ times). It's not good for readability to have to jump to `modeling_utils.py` and the code saved is not worth it for a one-liner.

Also, I'd advocate to make three separate PRs (one for PT, one for TF, one for Flax). Think it should be both easier to maintain the PRs as well as review them. 

A first test should then be that all slow tests pass. After that it would indeed be nice if we could run some fine-tuning for the most important models (BERT on GLUE, GPT2 on causal LM, T5 on translation maybe). Maybe also not even necessary to verify that everything is correct with a training run if the slow tests all pass 

Hi,

@patrickvonplaten:
  - I removed the new function.
  - I have to modify `FlaxT5Attention` otherwise the PT/Flax T5 equivalence tests will fail.

@sgugger:
  - since there is no more new function `mask_value()`, so no more `device` issue. There is one place I need to use tensor and device though:

 https://github.com/huggingface/transformers/blob/195ef42e0be974e8c019e9d5f03070f65365c721/src/transformers/models/gpt2/modeling_gpt2.py#L202-L205

Would this be a problem for model parallelism for big model inference? It is `attn_weights.device` instead of `self.dtype` though.

@ydshieh Using the weight device is perfectly fine, thanks for checking!

Cool, exciting!

Hi, @patrickvonplaten @patil-suraj @sgugger @LysandreJik 

This PR is ready for review. 

- Only dealing with PyTorch models: but need to change `FlaxT5` too to make the test pass.
- In general, change to `torch.finfo(correct-dtype).min` instead of `-10000`, `-1e9` etc.
- In particular, changes in `modeling_utils.py`
- Verified the change by training a T5 from scratch as well as finetuning the `t5-small` checkpoint

@sgugger @patrickvonplaten: sorry to call you after the approval, there is something more to discuss after I saw #17437 by @younesbelkada.

- First I need to remove the use of `float("-inf")` as done in [this commit](https://github.com/huggingface/transformers/pull/17306/commits/9fa9d9e8ac4aec9df3c42b3c6e6b178271230247)
  - It's better not to use `inf`: it is hard to track if there is any arithmetic ops that will produce `NaN`.

- Second, there are still some issues when using `torch.finfo(dtype).min`, especially when running in `fp16`, see the code snippet below.

  Basically: 
  - `torch.finfo(torch.float16).min + (-16.0) = -inf`. (or anything < `-16.0`)
  - In some cases, we get all 0s as attention mask:
    - for example, `[pad_token, token_1]` will give an attention mask `[[0, 0], [0, 1]]` (due to causal mask + padding).
    - this mask becomes `[[-65504, -65504], ...]` after using `torch.finfo(dtype).min` (for `fp16`)
    - This might give `[[-inf, -inf], ...]` when combining an `atten_scores` with values < -16.0 
    - Then, we get `NaN` after softmax. 

~~- `torch`'s `softmax` can't work with fp16 input on CPU~~
  - ~~RuntimeError: "softmax_lastdim_kernel_impl" not implemented for 'Half'~~

### Suggestions

~~- Cast `fp16` to `fp32` just before `softmax`, so it can run on CPU~~
- Perform some other processing before `softmax` to avoid `NaN` and non-sense output, especially avoid `[-inf, -inf]` or `[-inf, - large_value]` as input  to `softmax`
- Change attn probability to `[0, 0, .. 0]` if the input is all large negative value `torch.finfo(dtype).min` 

### Code Snippet
```
import torch
from torch import nn

# device = "cpu" --> not working with softmax on float16 (RuntimeError: "softmax_lastdim_kernel_impl" not implemented for 'Half')
device = "cuda"

dtype = torch.float16

mask_value = torch.finfo(dtype).min
attn_mask = torch.tensor([mask_value, mask_value], dtype=dtype)

attn_scores_0 = torch.tensor([-0, -0], dtype=dtype)
attn_scores_1 = torch.tensor([-4, -16], dtype=dtype)
attn_scores_2 = torch.tensor([-18, -16], dtype=dtype)

final_attn_scores_0 = attn_scores_0 + attn_mask  # --> [-65504,  -65504]
final_attn_scores_1 = attn_scores_1 + attn_mask  # --> [-65504,  -inf]
final_attn_scores_2 = attn_scores_2 + attn_mask  # --> [-inf,  -inf]

attn_prob_0 = nn.functional.softmax(final_attn_scores_0.to(device), dim=-1)  # --> [0.5, 0.5], but non-sense!!
attn_prob_1 = nn.functional.softmax(final_attn_scores_1.to(device), dim=-1)  # --> [1, 0], but non-sense!!
attn_prob_2 = nn.functional.softmax(final_attn_scores_2.to(device), dim=-1)  # --> [nan, nan], very bad!

print(final_attn_scores_0)
print(final_attn_scores_1)
print(final_attn_scores_2)
print(attn_prob_0)
print(attn_prob_1)
print(attn_prob_2)
```

Nothing should be done in FP16 on CPU, softmax is not the only operation that is not implemented in Half on CPU.

> Nothing should be done in FP16 on CPU, softmax is not the only operation that is not implemented in Half on CPU.

OK, thank you! Still want to hear your opinion on other points when you have more time

I'm personally leaning toward implementing an util that preprocesses the attention mask before the softmax as you suggested @ydshieh, but curious to see others opinion.

I think we can leave the discussion regarding `softmax` and  `attention score processing` to another thread and PR.

After removing `float("-inf")` in this PR, it's less likely to get all `-inf` in a particular sequence.
(It's sill likely to happen during training, so better to have a process done after this PR).

So I would prefer to merge the current version once @LysandreJik is happy with this PR.

@michaelbenayoun 

I have tried to keep your change 
```
mask = torch.full((tgt_len, tgt_len), torch.tensor(float("-inf")))
```
with my own, so it becomes
```
mask = torch.full((tgt_len, tgt_len), torch.tensor(torch.finfo(dtype).min))
```

If you encounter any problem after this PR is merged, don't hesitate to ping me.

@michaelbenayoun I need you help 🙏 


https://github.com/huggingface/transformers/blob/575a8c0a8e2bf6491d7ef0f932fb6caf8a7712b1/src/transformers/models/deberta_v2/modeling_deberta_v2.py#L135

I need to change `torch.tensor(float("-inf")))` to `torch.tensor(torch.finfo(dtype of an input).min))`

but I can't understand what the input it is here. Is it `g` or is it `self`.

From the line below
```
masked_fill(g, output, r_mask, g.op("Constant", value_t=torch.tensor(0, dtype=torch.uint8)))
```
I guess it might be the second argument, so I should use `self`, but it looks strange to me 😢 

@ydshieh About the first change you mention, I don't think it will break anything, and I will try to apply the same changes when I add support for new model architectures.

About the symbolic function, you are right I think. Basically `self` is `input` here, maybe we should change the name of the parameter to make things clearer?

> @ydshieh About the first change you mention, I don't think it will break anything, and I will try to apply the same changes when I add support for new model architectures.
> 
> About the symbolic function, you are right I think. Basically `self` is `input` here, maybe we should change the name of the parameter to make things clearer?

Great, thanks for the answer! We can change the name, but not very urgent.


Ran GPU non-slow tests (single/multi GPU) - results are fine.

Will merge today (as been approved by 3 core maintainers), if there is no further comment on

https://github.com/huggingface/transformers/pull/17306#pullrequestreview-994918858

As discussed on Slack, will wait the release of Bloom, and not merge now.