noamaanMulla-03/transformers-issues huggingface dataset:
T5 documentation for computing pretraining loss seems to have a mistake 
Author: dorost1234
Pull request: no

Dear @patrickvonplaten 

The documentation of T5 for computing loss of pretraining seems to have a mistake, where it talks on the loss formulation:
https://huggingface.co/transformers/model_doc/t5.html?highlight=decoder_input_ids 

```
input_ids = tokenizer('The <extra_id_0> walks in <extra_id_1> park', return_tensors='pt').input_ids
labels = tokenizer('<extra_id_0> cute dog <extra_id_1> the <extra_id_2>', return_tensors='pt').input_ids
# the forward function automatically creates the correct decoder_input_ids
loss = model(input_ids=input_ids, labels=labels).loss
```

1) the loss as per mt5 paper  "T5 is pre-trained on a masked language modeling “span-corruption” objective, where consecutive spans of input tokens are replaced with a mask token and the model is trained to **reconstruct the masked-out tokens.** "
https://arxiv.org/pdf/2010.11934.pdf

    So I believe you need to set the labels of masked tokens in labels to -100 then compute the loss in this example for pretraining.

2) Based on other examples in the repository, I think one need to do shift_right to compute the correct ` decoder_input_ids` before modifying the labels for the loss (before replacing -100 for the masked tokens) and pass it to the model as well, please correct me if mistaken. 

I greatly appreciate updating the documentations to show the correct procedure.

thanks. 

Comments:
Hey @dorost1234,

you're right to be fully correct we should set the `<extra_id_0>` to `-100`. Maybe we could add the following lines to the doc

```python
# make sure no loss is computed on sentinel tokens. Sentinel tokens are in `additional_special_tokens`.
labels = [label if label not in tokenizer.additional_special_tokens else -100]
```

Would you like (and have the time) to open a PR for this to improve the docs maybe? Otherwise I can do it as well! :-) 

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.
ONNX Training for Transformers
Author: harsha070
Pull request: no

# 🚀 Feature request

Is there a script for ONNX training of transformers on glue tasks ? If so, did anyone benchmark the training times ? If not, I can contribute.

## Motivation

Faster training.


Comments:
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.

Is it possible to pretrain bert with ONNX?

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.
generate text with inputs_embeds (instead of input_ids) for T5.
Author: nrjvarshney
Pull request: no

model.generate() supports input_ids only

```
outs = model.model.generate(input_ids=batch['source_ids'],
                                    attention_mask=batch['source_mask'],
                                    output_scores=True,
                                    max_length=model.model_arguments.max_output_seq_length)
        preds_cleaned = [model.tokenizer.decode(ids, skip_special_tokens=True, clean_up_tokenization_spaces=True)
                         for ids in outs]
```

It would be good to have the functionality of generating text from embeddings.
model.forward() allows passing inputs_embeds instead of input_ids. 


Comments:
One possible solution is to get the `encoder_outputs` by passing `inputs_embeds` to `encoder` and then passing that `encoder_outputs` to `.generate`, so for example

```
with torch.no_grad():
    encoder_outputs = model.get_encoder()(inputs_embeds=input_embeds)

gen_ids = model.generate(input_ids=None, encoder_outputs=encoder_outputs)
```

Thanks.
Faster list concat for trainer_pt_utils.get_length_grouped_indices()
Author: ctheodoris
Pull request: yes

# What does this PR do?

substitutes faster list concatenation for get_length_grouped_indices() in LengthGroupedSampler and DistributedLengthGroupedSampler as prior sum(megabatches, []) is prohibitively slow for large number of megabatches (in test case takes hours for ~270k megabatches with 100 items each).

Fixes #11795

## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [X] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [X] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@sgugger


Comments:
No problem, thank you for all your wonderful work!
Replace double occurrences as the last step
Author: LysandreJik
Pull request: yes

This PR fixes an issue with the SPM converters (ALBERT and XLNet) where it would replace some characters by whitespace - after removing double whitespace occurrences. This meant that if double whitespace were to appear thanks to this replacement, they would be kept until the end of encoding, leading to a mismatch between SentencePiece and `tokenizers`.

Fixes https://github.com/huggingface/transformers/issues/11358

Comments:
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.
[docs] XLnet reference link bug in description of past_index Parameter of TrainingArguments 
Author: Muktan
Pull request: no

XLnet reference link bug in description of past_index Parameter of TrainingArguments 
link to the doc: https://huggingface.co/transformers/main_classes/trainer.html#transformers.TrainingArguments

**current description:** Some models like TransformerXL or **:doc`XLNet <../model_doc/xlnet>`** can make use of the past hidden states for their predictions. If this argument is set to a positive int, the Trainer will use the corresponding output (usually index 2) as the past state and feed it to the model at the next training step under the keyword argument mems.

**expected description:** Some models like TransformerXL or **XLNet** can make use of the past hidden states for their predictions. If this argument is set to a positive int, the Trainer will use the corresponding output (usually index 2) as the past state and feed it to the model at the next training step under the keyword argument mems.

## Environment info
Not required
<!-- 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! -->

### Who can help
@sgugger
<!-- Your issue will be replied to more quickly if you can figure out the right person to tag with @
 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.


Comments:
Thanks for flagging! Should be fixed by the PR linked above!
Request addition of 'GPT2ForwardBackward' models
Author: GenTxt
Pull request: no
Labels: name: New model

# 🌟 Request addition of 'GPT2ForwardBackward' models

## Model description

Code for running forward and backward versions of GPT-2 XL. These were trained for the paper:

**Reflective Decoding: Beyond Unidirectional Generation with Off-the-Shelf Language Models**; Peter West, Ximing Lu, Ari Holtzman, Chandra Bhagavatula, Jena Hwang, and Yejin Choi; ACL (2021) 

https://arxiv.org/abs/2010.08566

## Open source status

* [ X] the model implementation is available: (https://github.com/peterwestuw/GPT2ForwardBackward)
* [ X] the model weights are available: (same link as above)
* [ X] who are the authors: (See arvix credits above)


Comments:
gpt-neo 2.7 crashes, 1.3 runs fine
Author: caseybasichis
Pull request: no

Loading the generator crashes Python

```
python3
Python 3.8.5 (default, Jan 27 2021, 15:41:15) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from transformers import pipeline
>>> generator = pipeline('text-generation', model='EleutherAI/gpt-neo-2.7B', device=0)
[2]+  Killed                  python3
Killed
```

I have an A6000 with 48gb.
... it looks like I'm running on 16gb of ram?? maybe a stick is dead -- usually 32gb.

Is it a system ram issue?

## Environment info
- `transformers` version: 4.5.0
- Platform: Linux-5.4.0-70-generic-x86_64-with-glibc2.29
- Python version: 3.8.5
- PyTorch version (GPU?): 1.9.0.dev20210217+cu112 (True)
- Tensorflow version (GPU?): not installed (NA)
- Using GPU in script?: <fill in>
- Using distributed or parallel set-up in script?: <fill in>


Comments:
I would guess that's a memory issue indeed! That should be the only difference between the two checkpoints.

Oof makes sense.  I'll check back if that doesn't fix it.  Thanks!

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.
Wav2vec2: comparison to original implementation
Author: cceyda
Pull request: no

# 🚀 Reproducibility challenge 

During the fine-tuning week I realized some of the smaller details in the implementation are a bit different than the original fairseq implementation. @patrickvonplaten 

Original Code: https://github.com/pytorch/fairseq/blob/master/fairseq/models/wav2vec/wav2vec2.py
🤗 Code: https://github.com/huggingface/transformers/blob/master/src/transformers/models/wav2vec2/modeling_wav2vec2.py

For example in `compute_mask_indices` function comparing scripts:
- There are naming differences, not that important as long as it is *documented* somewhere(?) for people transitioning to 🤗 & trying to replicate with the same hyper-parameters.
    `mask_time_prob=mask_prob (in fairseq)`
    `mask_time_length=mask_length(in fairseq)`
    `mask_feature_prob=mask_channel_length(in fairseq)`
    `mask_feature_length=mask_channel_length (in fairseq)`
   Also with the naming of different dropouts.

- But there are also seem to be some un-specified parameters:
`no_overlap`,`min_space`.
https://github.com/pytorch/fairseq/blob/05b86005bcca0155319fa9b81abfd69f63c06906/fairseq/models/wav2vec/wav2vec2.py#L348

Not sure how these effect, didn't look deeply, wanted to report just in case.

## Motivation

I realized while digging deeper because there was minor differences on results while finetuning (might be also be a fairseq problem https://github.com/pytorch/fairseq/issues/1448)

## Your contribution

opening this issue 😛 


Comments:
Hey @cceyda - thanks for the issue!

Yes, this was an intentional choice since those parameters seemed to be the same in all experiments after studying the paper. If people want to change those params, we could always add them in a later version.

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.
`decoder` without bias in BertLMPredictionHead
Author: bryant1410
Pull request: no

## ❓ Questions & Help

What does this comment mean?

https://github.com/huggingface/transformers/blob/80bf868a268fa445926bc93f7fe15960853e828e/transformers/modeling_bert.py#L394-L407

Comments:
Hi, I believe that means that the decoder is a linear layer that has the same weights as the word embedding matrix. However, that embedding matrix does not have a bias, whereas the decoder does have a bias. 

It is initialized to a vector of zeros here, but it can update its weights during training and has actual values in pre-trained models. For example:

```py
from transformers import BertForMaskedLM

bert = BertForMaskedLM.from_pretrained("bert-base-cased")
print(bert.cls.predictions.bias)

# tensor([-0.1788, -0.1758, -0.1752,  ..., -0.3448, -0.3574, -0.3483], requires_grad=True)
```

Oh, I see. That makes sense. So it should share parameters with the embedding weights? Where is that enforced in the code?

Yes, exactly. You can see it in the [bert_modeling.py file, inside the BertForMaskedLM class](https://github.com/huggingface/transformers/blob/master/transformers/modeling_bert.py#L754-L759).

Thanks for the clarification!

hi, 
I know it's an old issue but I had the same questions:
* it's not clear to me in the code where this parameter sharing is enforced
* is there any intuition why it is done ?  

thanks in advance 

@thibault-formal Hey, I had similar questions and asked them in the huggingface forum, I think my [post there](https://discuss.huggingface.co/t/understanding-bertlmpredictionhead/3618) could be helpful for you (if its still relevant). Your first point should be adressed by the explanation of my understanding and the second one is adressed by both replies.
Cheers!
Fix flos single node
Author: TevenLeScao
Pull request: yes

This PR fixes a bug typo where in single-node settings, flos in the Trainer would stay constant, and also updates the flo count in the trainer state at every log occasion (instead of every model-saving occasion) so that users that wish to use a flo-logging callback can access it more frequently. I feel like the first bug should have been caught by a test (at the moment few other people use trainer flos, since it's mostly for large-scale training and researchers that need or want to report flos, and neither group uses the Trainer a lot, so it's mostly the HF bigscience efforts), and I should make one.

@sgugger 

Comments:
Not sure why the CI is failing, is it related to the PR? Doesn't look so to me but I may be missing something

No, the CI is failing all the time those days, don't worry about it.
Fix two typos in docs
Author: nickls
Pull request: yes

# What does this PR do?
Fixed two minor typos.

## Before submitting
- [X] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [ ] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?
@sgugger



Comments:
Multi-node training with the latest transformers/examples code
Author: phqtuyen
Pull request: no

Hi,

I am trying to follow the instructions on how to use the examples from [https://huggingface.co/transformers/examples.html] and I notice there is a difference between version 4.3 and 1.2 in the distributed training session. In the older version it seems that it supports multi-node training with 
"    --node_rank=$THIS_MACHINE_INDEX \
    --master_addr="192.168.1.1" \
    --master_port=1234 run_bert_classifier.py \"
  But these options no longer exist in the latest tutorial. Does the latest version still support multi-node training? Thanks.

Comments:
cc @sgugger 

I'm unsure what you think is not supported. Launching any of the example scripts with 
```
python -m torch.distributed.launch --nproc_per_node=xxx \
   --node_rank=$THIS_MACHINE_INDEX \
   --master_addr="192.168.1.1" \
   --master_port=1234 \
   run_xxx.py
```
is going to work.

Hi @sgugger , thank you for your answer. My understanding is that "--nproc_per_node" is the number of gpus will be used for the launched process? Also, if I want to launch another training node, I assume I will just run the same command with "node_rank=1"?

Yes, that is the number of GPUs.

You can refer to the [PyTorch documentation](https://pytorch.org/docs/stable/distributed.html#launch-utility) for all the arguments of the PyTorch launcher as all the example scripts are fully compatible with it. You will also need to pass `--nnodes=$NUMBER_OF_NODES` for completeness.

Thanks, I am able to make it work now.
Multilingual MobileBERT
Author: Wikidepia
Pull request: no
Labels: name: New model

# 🌟 New model addition

## Model description
 
Recently Google released multilingual MobileBERT on [tensorflow/models](https://github.com/tensorflow/models/blob/master/official/nlp/projects/mobilebert/README.md)
>  In addition, we also provide new multiple-lingual MobileBERT checkpoint trained using multi-lingual Wiki data.

<!-- Important information -->

## Open source status

* [x] the model implementation is available: [tensorflow/models](https://github.com/tensorflow/models/blob/master/official/nlp/projects/mobilebert/README.md)
* [x] the model weights are available: Yup, [here](https://storage.cloud.google.com/tf_model_garden/official/mobilebert/multi_cased_L-24_H-128_B-512_A-4_F-4_OPT.tar.gz)
* [ ] who are the authors: (mention them, if possible by @gh-username)


Comments:
Add some tests to the slow suite 
Author: LysandreJik
Pull request: yes

This PR adds the torchscript tests to the slow suite. The current CI isn't passing because it crashes and exceeds the 10-minute timeout, this PR is a first step into fixing that.

Will look into re-enabling the torchscript tests on each commit (they'll be tested every day for now) once we refactor the test suite to be less hungry.

Comments:
[Demo] Slow down in TPU training
Author: patrickvonplaten
Pull request: yes

@avital @marcvanzee - I wanted to align `run_mlm_flax.py` more with `run_glue_flax.py` and noticed that by doing the change as shown in this PR, training on TPU slows down very significantly by ca. ~40%.

Currently, [`run_glue_flax.py`](https://github.com/huggingface/transformers/blob/master/examples/flax/text-classification/run_flax_glue.py) and [`run_mlm_flax.py`](https://github.com/huggingface/transformers/blob/master/examples/flax/language-modeling/run_flax_mlm.py) deal slightly differently with the PRNG key: `run_mlm_flax.py` splits the key inside the training step while `run_glue_flax.py` does so before the train step and shards it then before passing it to the train loop. It seems that `run_mlm_flax.py` is significantly faster on TPU.

Do you by any chance have good explanations for that? 

Comments:
Basically prior to this change you were running a single jitted function in each train step, and because of asynchronous dispatch it didn't have to wait until the previous step was complete until dispatching the program for the next step. But if you split an RNG in between, then JAX blocks until the previous step was complete, then dispatches and executes the split command, and then dispatches the next training step.

In short, the guideline is that each step in a training loop should be a single jitted function. If done right this should lead to close to 100% device utilization.

This is a common gotcha -- people hit this regularly, and we should help catch the slow patterns early, such that you could detect this even with a local run with no accelerator/unit test. @jheek is working on a library that would allow you to annotate code such that you'd get that kind of error or warning for this, and other cases.

@jheek also said:

Yeah this is an example of my number 1 most common and most hurtful JAX performance gotcha that I want to catch automatically

In this case it stands out but there are more subtle variants where it's hard to spot in a review

This analysis is only true for TPU without async mode enabled btw. Because all other devices have a queue that is > 1


Thanks a lot for this in-detail explanation @avital! 

Also pinging @sgugger @stas00 @mfuntowicz - might be interesting to read :-) 

(I guess really this means that `run_glue_flax.py` could be made faster? /cc @marcvanzee )

> (I guess really this means that `run_glue_flax.py` could be made faster? /cc @marcvanzee )

Yeah, I'm currently testing it actually, see here: https://github.com/huggingface/transformers/pull/11778 .
Will report results tomorrow

Great that you discovered this! I actually didn't notice the bug, and since training was already fast enough I didn't look into it. Curious to see whether we will get even more speedup!

Reran, the experiments - got a small speed-up on TPU. Here the new numbers: https://github.com/huggingface/transformers/tree/master/examples/flax/text-classification#runtime-evaluation