Fix -1e4 as attn mask

[ydshieh]

What does this PR do?

Fix the issues regarding -1e4 as attention mask.

Fix #17215 #17121 #14859

[HuggingFaceDocBuilderDev]

The documentation is not available anymore as the PR was closed or merged.

[sgugger]

The PyTorch implementation relies on self.device which breaks the model parallelism for big model inference, so we should avoid using it (I actually removed lots of instance where we used it recently, and will hunt the other ones in another PR ;-) )

[patrickvonplaten]

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

[ydshieh]

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:

transformers/src/transformers/models/gpt2/modeling_gpt2.py

Lines 202 to 205 in 195ef42

	mask_value = torch.finfo(attn_weights.dtype).min
	# Need to be a tensor, otherwise we get error: `RuntimeError: expected scalar type float but found double`.
	# Need to be on the same device, otherwise `RuntimeError: ..., x and y to be on the same device`
	mask_value = torch.tensor(mask_value, dtype=attn_weights.dtype).to(attn_weights.device)

Would this be a problem for model parallelism for big model inference? It is attn_weights.device instead of self.dtype though.

[patil-suraj]

It looks good in general. I have pretty much the same comments as Patrick. I would advocate to do some fine-tuning even if the slow tests pass to make sure it doesn't break anything. Especially with models like T5 which have had issues with attention_mask.

[sgugger]

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

[LysandreJik]

Cool, exciting!

[ydshieh]

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]

Thanks for fixing all of those!

[patrickvonplaten]

Great work @ydshieh ! Looks good to me