Wav2Vec2ForPreTraining in 4.12 broke SpeechBrain implementation
Author: TParcollet
Pull request: no

## Environment info

- `transformers` version:
- Platform: Linux
- Python version: 3.8
- PyTorch version (GPU?): 1.9 (and 1.10)
- Using GPU in script?: 1-32 Tesla V100
- Using distributed or parallel set-up in script?: DDP

### Who can help
@patrickvonplaten, @anton-l

## Information

Model I am using (Bert, XLNet ...): wav2vec-base (original is on facebookai repo)

The problem arises when using:
* [ ] the official example scripts: (give details below)
* [x] my own modified scripts: (give details below)

The tasks I am working on is:
* [ ] an official GLUE/SQUaD task: (give the name)
* [x] my own task or dataset: (give details below)

## To reproduce

Steps to reproduce the behavior:

1. Go to the [SpeechBrain PR](https://github.com/speechbrain/speechbrain/pull/1062) and use the corresponding branch
2. Install speechbrain (pip install -r requirements.txt / pip install -e .) 
3. Install extra_requirements in recipes/CommonVoice/self-supervised-learning/wav2vec2/extra_requirements.txt)
4. Download and untar any CommonVoice english version (best using an old one to get less hours to debug ...)
5. start the training with a single GPU (as it doesn't work either anymore) with: `python recipes/CommonVoice/self-supervised-learning/wav2vec2/train.py recipes/CommonVoice/self-supervised-learning/wav2vec2/hparams/wav2vec2_base.yaml --data_folder=/path/to/CV/en --batch_size=adapttoyourgpu(12 if 32GB) --gradient_accumulation=8 or 16`

## Extra information about the code

The important code can be located in `recipes/CommonVoice/self-supervised-learning/wav2vec2/train.py` under the brain class for compute_forward and compute_objectives. The entire wrapping of the HF model into SpeechBrain happens at the bottom of the `speechbrain/lobes/models/hugginface_wav2vec2.py` file. 

The batch that is received simply is of the form (batch, signal) just like for HF.  

## Expected behavior

With 4.11 (code can be found in the same PR from earlier commit) everything was working well ! We were even able to submit papers based on this work. Here is a list of the different logs obtained with the old working version:
```
epoch: 1, lr: 1.87e-05, steps: 1027, optimizer: AdamW - train loss: 6.41e+03 - valid loss: 4.53e+03, valid acc: 0.14673814177513123
epoch: 2, lr: 3.75e-05, steps: 2054, optimizer: AdamW - train loss: 6.18e+03 - valid loss: 4.45e+03, valid acc: 0.21184375882148743
epoch: 3, lr: 5.62e-05, steps: 3081, optimizer: AdamW - train loss: 5.67e+03 - valid loss: 3.70e+03, valid acc: 0.26702988147735596
epoch: 4, lr: 7.50e-05, steps: 4108, optimizer: AdamW - train loss: 5.19e+03 - valid loss: 3.70e+03, valid acc: 0.301466703414917
epoch: 5, lr: 9.37e-05, steps: 5135, optimizer: AdamW - train loss: 5.15e+03 - valid loss: 3.58e+03, valid acc: 0.33249199390411377
epoch: 6, lr: 1.12e-04, steps: 6162, optimizer: AdamW - train loss: 5.05e+03 - valid loss: 3.49e+03, valid acc: 0.3265174329280853
```
Now, we the new implementation:
```
epoch: 1, lr: 1.87e-05, steps: 1027, optimizer: AdamW - train loss: 7.09e+03 - valid loss: 4.87e+03, valid acc: 0.15861859917640686
epoch: 2, lr: 3.75e-05, steps: 2054, optimizer: AdamW - train loss: 6.67e+03 - valid loss: 4.67e+03, valid acc: 0.19915643334388733
epoch: 3, lr: 5.62e-05, steps: 3081, optimizer: AdamW - train loss: 6.39e+03 - valid loss: 4.41e+03, valid acc: 0.22449128329753876
epoch: 4, lr: 7.50e-05, steps: 4108, optimizer: AdamW - train loss: 6.18e+03 - valid loss: 4.25e+03, valid acc: 0.24435752630233765
epoch: 5, lr: 9.37e-05, steps: 5135, optimizer: AdamW - train loss: 6.01e+03 - valid loss: 4.15e+03, valid acc: 0.2056254893541336
epoch: 6, lr: 1.12e-04, steps: 6162, optimizer: AdamW - train loss: 5.88e+03 - valid loss: 4.11e+03, valid acc: 0.2493399679660797
epoch: 7, lr: 1.31e-04, steps: 7189, optimizer: AdamW - train loss: 5.76e+03 - valid loss: 4.02e+03, valid acc: 0.27252206206321716
epoch: 8, lr: 1.50e-04, steps: 8216, optimizer: AdamW - train loss: 5.66e+03 - valid loss: 3.97e+03, valid acc: 0.26998990774154663
epoch: 9, lr: 1.69e-04, steps: 9243, optimizer: AdamW - train loss: 5.59e+03 - valid loss: 3.85e+03, valid acc: 0.24951176345348358
epoch: 10, lr: 1.87e-04, steps: 10270, optimizer: AdamW - train loss: 5.51e+03 - valid loss: 3.80e+03, valid acc: 0.24127712845802307
epoch: 11, lr: 2.06e-04, steps: 11297, optimizer: AdamW - train loss: 5.43e+03 - valid loss: 3.72e+03, valid acc: 0.2344648540019989
epoch: 12, lr: 2.25e-04, steps: 12324, optimizer: AdamW - train loss: 5.37e+03 - valid loss: 3.74e+03, valid acc: 0.20351676642894745
epoch: 13, lr: 2.44e-04, steps: 13351, optimizer: AdamW - train loss: 5.30e+03 - valid loss: 3.72e+03, valid acc: 0.1984717845916748
epoch: 14, lr: 2.62e-04, steps: 14378, optimizer: AdamW - train loss: 5.29e+03 - valid loss: 3.66e+03, valid acc: 0.2088804990053177
epoch: 15, lr: 2.81e-04, steps: 15405, optimizer: AdamW - train loss: 5.25e+03 - valid loss: 3.64e+03, valid acc: 0.21932080388069153
epoch: 16, lr: 3.00e-04, steps: 16432, optimizer: AdamW - train loss: 5.21e+03 - valid loss: 3.62e+03, valid acc: 0.20787915587425232
```

As a side not, I think that exporting masking and negative_sampling from the forward function is a bad idea for external toolkit compability. If everything was ambedded in the .forward() function, any toolkit could just instantiate your model and run it without worrying about the library version. Now, everytime HuggingFace generates a new transformers version, I will have to check and adapt the potential changes :-(



Comments:
I printed the mask_indices obtained by calling your function and it looks correct. Is it normal however that the call to `transformers.models.wav2vec2.modeling_wav2vec2._sample_negative_indices` returns this (batch_size being 6): 
<img width="481" alt="Capture d’écran 2021-11-20 à 18 51 12" src="https://user-images.githubusercontent.com/11910731/142736330-77e6001b-e756-4a0c-9104-54ccd1404245.png">



Ok I took a look at the differences between the two scripts - there are **two** main differences (which were bugs in 4.11 IMO).

1)
The diversity loss was completely incorrectly scaled in 4.11 - see: https://github.com/huggingface/transformers/blob/dc193c906dfb3b9663f8963735c46e030a15b914/src/transformers/models/wav2vec2/modeling_wav2vec2.py#[…]3 here you can see that while the contrastive loss is scaled by the number of target (quantized) vectors, the diversity loss is not multiplied by the number of target vectors. On master this has been corrected: https://github.com/huggingface/transformers/blob/6fc38adff272ea3148e05888edf67eeb00170453/src/transformers/models/wav2vec2/modeling_wav2vec2.py#[…]0

This was one of the major resaons why the training became very unstable for me - since the diversity loss was incorrectly scaled the quantization codebook vectors always collapsed

If in your case diversity loss seems to be very much unnecessary you could try scaling it down by a factor of 10 or more using current master to have more or less the same loss as in 4.11

2. (That's a big one) in 4.11 the negative quantized vectors were taken from all possible quantized input vectors - which can be seen here: https://github.com/huggingface/transformers/blob/dc193c906dfb3b9663f8963735c46e030a15b914/src/transformers/models/wav2vec2/modeling_wav2vec2.py#[…]7 as the attention_mask is passed as the mask

However this is not how the original fairseq models were trained

the original Wav2Vec2 models were trained using only target vectors (so the subset of all quantized input vectors as defined by mask_time_indices) as negative vectors

This is now also done on master: https://github.com/huggingface/transformers/blob/6fc38adff272ea3148e05888edf67eeb00170453/src/transformers/models/wav2vec2/modeling_wav2vec2.py#L229

The authors of wav2vec2 told me that they tried both the 4.11 and the master approach and master always led to better final WER fine-tuning results while the 4.11 approach was more robust


You could revert back to the original change by simple passing sampled_negative_indices=attention_mask here: https://github.com/huggingface/transformers/blob/6fc38adff272ea3148e05888edf67eeb00170453/src/transformers/models/wav2vec2/modeling_wav2vec2.py#L228 and see what happens

Those were the main changes between 4.11 and master which IMO are both bugfixes

Maybe a good start would be to:
a) Use a tiny diversity_loss weight and
b) generate the negative_sample_indices by passing the attention_mask instead of mask_time_indices



> I printed the mask_indices obtained by calling your function and it looks correct. Is it normal however that the call to `transformers.models.wav2vec2.modeling_wav2vec2._sample_negative_indices` returns this (batch_size being 6): <img alt="Capture d’écran 2021-11-20 à 18 51 12" width="481" src="https://user-images.githubusercontent.com/11910731/142736330-77e6001b-e756-4a0c-9104-54ccd1404245.png">

yeah this looks correct

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.

@TParcollet - everything is fine now with the pretraining no? Or is there anything else that should be fixed? 

It is partially fine. We had to revert to the whole sentence sampling due to small sentences

Ok! And now having replaced `mask_time_indices=mask_time_indices` with `mask_time_indices=torch.ones(...)` fixed the problem or still not 100%? 

It's fixed, but mysterious research-wise.