noamaanMulla-03/transformers-issues huggingface dataset:
[T5] Fix speed degradation bug t5
Author: patrickvonplaten
Pull request: yes

# What does this PR do?


Checking every value of a tensor for `inf` is expensive. This was added to T5 to allow for fp16 training, but should then also be used when the model is in fp16 to not slow down normal fp32 mode.

Using @dsgissin  script:

```python
device = torch.device('cuda:0') if torch.cuda.is_available() else torch.device('cpu')
print(f"Using device: {device}")

t5_tokenizer = T5TokenizerFast.from_pretrained('t5-base')
t5_model = T5ForConditionalGeneration.from_pretrained('t5-base')
t5_model = t5_model.to(device)

t5_input_ids = t5_tokenizer("summarize: studies have shown that owning a dog is good for you ", return_tensors="pt").input_ids  # Batch size 1
t5_input_ids = t5_input_ids.to(device)

import time
import numpy as np
N = 100
times = []
for _ in range(N):
  start = time.time()
  t5_outputs = t5_model.generate(t5_input_ids)
  end = time.time()
  times.append(end-start)
print(f"transformers version: {transformers_version}")
print(f"torch version: {torch_version}")
print(f"{1000*np.mean(times):.0f} ms \u00B1 {1000*np.std(times):.2f} ms per loop (mean \u00B1 std of {N} runs)")
```

with:

- Python 3.8.5
- PyTorch 1.7.1
- CUDA 11.1 on a NVIDIA V100 GPU

The time was improved from:
```441 ms ± 41.67 ms per loop (mean ± std of 100 runs)```
to 
```388 ms ± 44.75 ms per loop (mean ± std of 100 runs)```

Comments:
> Looks good to me!
> 
> Some of the other library models also use this trick (BART-like models), we should also investigate those.

Good point - yeah, let me fix this in this PR actually
Refactor checkpoint name in BERT and MobileBERT
Author: sgugger
Pull request: yes

# What does this PR do?

Linked to #10193, this PR gives an example on how to refactor the checkpoint names in one private constant and use the `# Copied from` syntax to make the task-specific models that are copies of each other properly watched by our tooling.

It adds the option in check-copies to:
- put multiple statements behind the with: so for instance `with Bert->MobileBert, bert->mobilebert, BERT->MOBILEBERT`
- have the option to do all possible casings (like in the example above) by just adding `all-casing`:  `with Bert->MobileBert all-casing`.

It also fixes an existing bug when the first line of the function/class copied from was empty.



Comments:
remap MODEL_FOR_QUESTION_ANSWERING_MAPPING classes to names auto-generated file
Author: stas00
Pull request: yes

As discussed in https://github.com/huggingface/transformers/issues/10467 currently Trainer loads `models.auto.modeling_auto` which loads **all** modeling files, which is absolutely not needed most of the time. At times it could have unwanted side-effects, loading modeling files that require some 3rd party modules that could be a problem.

Similar to the autogenerated 3rd party version numbers look up dict, this PR autogenerates the names of the classes dict for `MODEL_FOR_QUESTION_ANSWERING_MAPPING` which can then be quickly loaded.

This of course can be extended in the future to generate other structures if need be.

@sgugger, @LysandreJik 

Fixes: https://github.com/huggingface/transformers/issues/10467


Comments:
The described function in docs was not implemented in source code
Author: yww211
Pull request: no

https://github.com/huggingface/transformers/blob/0c2325198fd638e5d1f0c7dcbdd8bf7f14c0ff7d/src/transformers/file_utils.py#L1512

The method of `update` in `ModelOutput` described as below,
https://huggingface.co/transformers/main_classes/output.html?highlight=modeloutput#transformers.file_utils.ModelOutput.update
but the source code will raise an exception when try to use it, which disagree with the docs.

Comments:
Ah, indeed! cc @sgugger
Checkpoint refactoring for Multiple Models
Author: danielpatrickhug
Pull request: yes

Hi, thank you for providing an example @sgugger 
Linked to #10193, this PR refactors the checkpoint names in one private constant. 
A couple notes:
- I refactored most of the modeling files, however I excluded the modeling_tf_*.py files for now.
- The bare Distilbert foward pass has two add_code_sample_docstrings decorators, I wanted to check for confirmation that its redundant.
- funnel, gpt and squeeze bert all have 2 checkpoint models for different tasks, so I left those alone.





Fixes #10193


## 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?




Comments:
> Thanks a lot for the PR!
> 
> We can do the TF models in another PR, I'm completely fine with that. Regarding your other comments:
> 
> * funnel is a special case indeed, so it's fine to leave it as it for now.
> * for squeezebert, you can just use "squeezebert/squeezebert-uncased" everywhere
> * for gpt I didn't see two checkpoints, just one.
> * for distilbert, the duplicate `add_code_ample` decorator should be removed.
> 
> One last thing, you added a `datasets` submodule in your PR that you should remove before we can merge it.

oo sorry about that Ill remove the datasets and clean up the rest, so GPT2 has 2 checkpoints("gpt2" and for sequence classification..
```
    @add_code_sample_docstrings(
        tokenizer_class=_TOKENIZER_FOR_DOC,
        checkpoint="microsoft/dialogrpt",
        output_type=SequenceClassifierOutputWithPast,
        config_class=_CONFIG_FOR_DOC,
    )
```

Ok for GPT2, you can leave this checkpoint and just refactor the traditional "gpt2" in the other places then.

Oh no, it looks like the rebase messed with GitHub and the diff. Do you think you could close this PR and open a fresh one on your branch?

Also, for the next step #10424 contains an example of what I was envisioning for the "# Copied from", if it still interests you to work on this part in a second stage. The PR needs to be merged before you work on it because there is some fixes in our check_copies script inside.

 sure no problem, Ill include the TF modeling files as well in the next pull request.
[DeepSpeed] unable to increase batch size from 4 for T5-3b with 2x 32GB V100 GPUs
Author: saichandrapandraju
Pull request: no

Hi,

I'm trying T5-3b with DeepSpeed on 2 V100-32GB GPU's. But I'm unable to increase batch size beyond 4 with max i/p sequence length of 512 and max o/p sequence length of 4. 
Previously I tried with t5.parallelize() [ i.e, without DeepSpeed ] on same setup and was able to train with batch size of 2. 

Below are my training args and DeepSpeed's config - 
```
training_args = Seq2SeqTrainingArguments(
    output_dir='./seq_out/results',
    overwrite_output_dir=True,
    evaluation_strategy="epoch",
    per_device_train_batch_size=4,
    per_device_eval_batch_size=4,
    learning_rate=3e-5,
    weight_decay=0.01,
    num_train_epochs=2,    
    warmup_steps=500,             
    logging_dir='./seq_out/logs',          
    logging_steps=10,
    load_best_model_at_end=True,
    deepspeed='ds_config.json'
)
```
```
{
    "fp16": {
        "enabled": true,
        "loss_scale": 0,
        "loss_scale_window": 1000,
        "hysteresis": 2,
        "min_loss_scale": 1
    },

   "zero_optimization": {
       "stage": 2,
       "allgather_partitions": true,
       "allgather_bucket_size": 1.5e8,
       "overlap_comm": true,
       "reduce_scatter": true,
       "reduce_bucket_size": 1.5e8,
       "contiguous_gradients": true,
       "cpu_offload": true
   },

   "zero_allow_untested_optimizer": true,

   "optimizer": {
     "type": "AdamW",
     "params": {
       "lr": 3e-5,
       "betas": [
         0.8,
         0.999
       ],
       "eps": 1e-8,
       "weight_decay": 3e-7
     }
   },

   "scheduler": {
     "type": "WarmupLR",
     "params": {
       "warmup_min_lr": 0,
       "warmup_max_lr": 3e-5,
       "warmup_num_steps": 500
     }
   },

    "steps_per_print": 2000,
    "wall_clock_breakdown": false
}
```

Observed similar behavior with T5-large as well . I was able to train with 14 batch size ( same i/p, o/p seq length and config.json as above ) on single GPU ( 32 GB V100 ) BUT when executing the same on 2x 32 GB GPUs, I was not able to go beyond 14 batch size( which I was able to train with 1 GPU itself) and memory from both the GPUs was consumed (31 GB and 28 GB).

Reducing `allgather_bucket_size ` and `reduce_bucket_size` didn't help in increasing batch size.

But I expected more batch size with DeepSpeed and CPU offloading. Is this fine or am I making something wrong which is hindering deepspeed's capability..?


Comments:
[BERT] Implementation of the sliding window for long sequences
Author: wasiahmad
Pull request: no
Labels: name: wontfix

I was trying to look for the references where the sliding window is implemented to process long sequences. How do we split a long sequence and then after getting the embeddings, how do we unpack them? I am unable to find the code segments that handle these operations.

Also, is it possible to describe the main trick? I am trying to implement it in plain PyTorch. I am unable to implement in Batches without running any loops.

Any help would be appreciated.

Comments:
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.


I have the same issue.. anyone has any inputs on this
Not always consider a local model a checkpoint in run_glue
Author: sgugger
Pull request: yes

# What does this PR do?

In the `run_glue` script, a local model is automatically considered a checkpoint (which is there to enable users to do --model_path_or_name path_to_specific_checkpoint`) but when using a local model, it can crash if the number of labels is different (cf #10502). This PR fixes that by checking the number of labels inside the potential checkpoint before passing it to the `Trainer`. Another possible fix is to check for the presence of files like `trainer_state.json` (but this only works if the checkpoint was created with a recent version of Transformers).

Fixes #10502 


Comments:
Removes overwrites for output_dir 
Author: philschmid
Pull request: yes

# What does this PR do?

This PR removes the overwrites of the `output_dir` when running training on `SageMaker` and it removes the automatic save if `output_dir` is `None`. 
The overwrites have been removed since it prevents saving checkpoints to a different dir like `opt/ml/checkpoints` impossible and it is not the "transformers" way. We can keep it the same as running training somewhere else and provide documentation on what you need to do.

Comments:
Rework TPU checkpointing in Trainer
Author: sgugger
Pull request: yes

# What does this PR do?

This PR rewors a tiny bit the checkpointing mechanism in the `Trainer` and `PreTrainedModel` to get rid of the hack that stored something in the config (which was then forever present if the user decided to share their model on the hub).

The main problem is that the save on TPU has to be called on all processes because there is a synchronization inside (so if we only execute it in one process, the other do not reach the synchronization point and everything hangs). At the same time, the config should only be saved on one process to avoid race conditions. So this PR solves the problem by adding two new arguments to `save_pretrained`: `is_main` and `_save_function`.

Comments:
Tested on TPUs and all worked well (training, checkpointing, reloading from checkpoint, evaluating a saved model), so I will merge this.
BERT as encoder - position ids
Author: md975
Pull request: no

Hello,

I have an EncoderDecoderModel.from_encoder_decoder_pretrained which is using BERT as both the decoder and encoder.
I would like to adjust position_ids for the encoder input, however, looking at [this documentation](https://huggingface.co/transformers/model_doc/encoderdecoder.html#transformers.EncoderDecoderModel.forward) it seems like there is no such argument.
How can I do this?
Sorry if this is an obvious question, I'm new to this stuff.
Thanks!

Comments:
Hello! In the documentation you pointed to, you'll see that there is the `kwargs` argument, which accepts any keyword argument. The doc says:

```
(optional) Remaining dictionary of keyword arguments. Keyword arguments come in two flavors:

Without a prefix which will be input as **encoder_kwargs for the encoder forward function.

With a decoder_ prefix which will be input as **decoder_kwargs for the decoder forward function.
```

Have you tried passing `position_ids` to the `__call__` method of your model instantiated with the `EncoderDecoderModel.from_encoder_decoder_pretrained`? It should work!

Thanks so much!
Addition of VisualBERT
Author: gchhablani
Pull request: no
Labels: name: wontfix

name: New model

# 🌟 New model addition
## Model description
The VisualBERT model is used for multi-modal processing when the modes of images and text are present. It takes in object detection features from images, and combines them with textual embeddings from the pre-trained BERT models, pre-trained the whole thing on COCO image captioning data, using a similar MLM task as BERT. It has been shown to work well on several multi-modal tasks such as VQA, VCR, NLVR, etc.

<!-- Important information -->

## Open source status

The source code presented along with the paper can be found at 
https://github.com/uclanlp/visualbert
* [x] the model implementation is available: (give details)
The model implementation can be found on the GitHub repository, in the models section: https://github.com/uclanlp/visualbert/tree/master/models
This code was provided along with the paper.
Another implementation, which is slightly harder to understand because of complex dependencies, is implemented in the Facebook Research's MMF framework: https://github.com/facebookresearch/mmf/blob/master/mmf/models/visual_bert.py

* [x] the model weights are available: (give details)
The model checkpoints that the authors used are presented as drive links in the given repository, depending on which pre-training we want. There are several links on the README file of the GitHub repository.
* [x] who are the authors: (mention them, if possible by @gh-username)
- Kai-Wei Chang: @KaiWeiChang
- Liunian Harold Li: @liunian-harold-li
- Mark Yatskar
- Da Yin
- Cho-Jui Hsieh
- Kai-Wei Chang

I want to contribute the model myself, please let me know if this is the right avenue for this, and how I can contribute.

Comments:
This is very interesting!

This has been proposed before as a separate issue but no action was taken. Hence, I thought I'll start implementing some of the multi-modal models one by one.

Please let @liunian-harold-li and me know if you need any help. We can also provide the pre-trained models. 

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
pass correct head mask to cross-attention layer
Author: patil-suraj
Pull request: yes

# What does this PR do?


 MBart, Blender, and Pegaus models decoder layers pass `layer_head_mask` head mask to cross attention layer, which is incorrect. This PR passes the correct `encoder_layer_head_mask` to cross-attention layer.

Comments:
I see, then this looks a bit confusing to me, other seq2seq models (BART, Marian, BlendSmall) pass `encoder_layer_head_mask` to cross attention

https://github.com/huggingface/transformers/blob/9248e27037ce7f7c9359802e6fdf819a1e227a18/src/transformers/models/bart/modeling_bart.py#L419

https://github.com/huggingface/transformers/blob/9248e27037ce7f7c9359802e6fdf819a1e227a18/src/transformers/models/marian/modeling_marian.py#L438 

https://github.com/huggingface/transformers/blob/9248e27037ce7f7c9359802e6fdf819a1e227a18/src/transformers/models/blenderbot_small/modeling_blenderbot_small.py#L421

> s looks a bit confusing to me, other seq2seq models (BART, Marian, BlendSmall) pass `encoder_layer_head_mask` to cross attention

I would argue that they are wrong then :D (cc @stancld - maybe do you have more insight here?)

@patil-suraj @patrickvonplaten - What a coincidence... I was discussing this topic with my PhD supervisor this morning and I think the most proper way how to handle head masking for cross attention is to introduce a separate cross-attention head mask tensor to disentangle the cross-attention effect from the self-attention one?

I guess this would actually be the cleanest option! At the moment the cross-attention layer the exact same shape as the decoder self-attention layer -> so I think we can use the same mask for now and maybe improve it later with a `cross-attention head mask`. Using the `encoder_layer_head_mask` however could lead to errors IMO - so this option is just wrong to me

I can create a new issue and can have a look at this cross-attention `head_mask` at the weekend :)

That would be great @stancld! I will close this PR.
Fixing conversation test for torch 1.8
Author: Narsil
Pull request: yes

# What does this PR do?

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->

Fixes # (issue)


## Before submitting
- [ ] 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?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors which may be interested in your PR.

<!-- Your PR 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.

Models:

- albert, bert, xlm: @LysandreJik
- blenderbot, bart, marian, pegasus, encoderdecoder,  t5: @patrickvonplaten, @patil-suraj
- longformer, reformer, transfoxl, xlnet: @patrickvonplaten
- fsmt: @stas00
- funnel: @sgugger
- gpt2: @patrickvonplaten, @LysandreJik
- rag: @patrickvonplaten, @lhoestq
- tensorflow: @jplu

Library:

- benchmarks: @patrickvonplaten
- deepspeed: @stas00
- ray/raytune: @richardliaw, @amogkam
- text generation: @patrickvonplaten
- tokenizers: @n1t0, @LysandreJik
- trainer: @sgugger
- pipelines: @LysandreJik

Documentation: @sgugger

HF projects:

- nlp datasets: [different repo](https://github.com/huggingface/nlp)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Examples:

- maintained examples (not research project or legacy): @sgugger, @patil-suraj
- research_projects/bert-loses-patience: @JetRunner
- research_projects/distillation: @VictorSanh

 -->

Comments:
fine-tune Pegasus with xsum using Colab but generation results have no difference
Author: harrywang
Pull request: no

Hi. I tried to fine-tune pegasus large with xsum dataset using Colab (Pro). I was able to finish the fine-tuning with batch size 1, and 2000 epochs in about 40 minutes (larger batch size crashed colab). The working Colab notebook I used is shared at https://colab.research.google.com/drive/1RyUsYDAo6bA1RZICMb-FxYLszBcDY81X?usp=sharing

However, the generated summary seems to be the same for the pegasus large model (https://huggingface.co/google/pegasus-large) and the fine-tuned model. But the generated result using pegasus xsum model (https://huggingface.co/google/pegasus-xsum) is different and much better. 

The training loss is already 0 and I am not sure what I have done wrong. Any help and pointers are highly appreciated. 

@sshleifer


Comments:
Hi,

could you please ask this question on the [forum](https://discuss.huggingface.co/)? We're happy to help you there!

Questions regarding training of models are a perfect use case for the forum :) for example, [here](https://discuss.huggingface.co/search?q=pegasus) you can find all questions related to fine-tuning PEGASUS.

@NielsRogge thanks a lot! I will post the question in forum 😄🤝

I think this issue can be closed - I used another input text and the generated text is different - I guess the fine-tuned model is different but for some input text, the generated result is exactly the same as the large model - interesting to know why though :).
Dead link to optuna.create_study under hyperparamter_search in Trainer
Author: joawar
Pull request: no

I noticed the hyperlink to the documentation of optuna's create_study under ```kwargs``` in the ```hyperparameter_search``` method of Trainer is outdated. 

https://huggingface.co/transformers/main_classes/trainer.html

### Who can help
Documentation: @sgugger

New URL (I'm guessing): https://optuna.readthedocs.io/en/stable/reference/generated/optuna.study.create_study.html
Old (currently used): https://optuna.readthedocs.io/en/stable/reference/alias_generated/optuna.create_study.html#optuna.create_study




Comments:
Thanks for flagging! Do you want to make a PR to fix it?

> Thanks for flagging! Do you want to make a PR to fix it?

I tried (#10554). Did I do it correctly?

Looks okay to me! Let's just wait to check the tests pass. Thanks! :-)
Typo in deberta_v2/__init__.py
Author: cliang1453
Pull request: no

https://github.com/huggingface/transformers/blob/c503a1c15ec1b11e69a3eaaf06edfa87c05a2849/src/transformers/models/deberta_v2/__init__.py#L31
Should be '' DEBERTA_V2_PRETRAINED_MODEL_ARCHIVE_LIST ''.

Comments:
That's correct! Do you want to open a PR to fix it?
Transfomer-xl  padding token
Author: Kouuh
Pull request: no

When dealing with a batch consisting of sequences of different lengths, how do I choose parameters so that padding_token is not involved in the computation.

Comments:
Hello, thanks for opening an issue! We try to keep the github issues for bugs/feature requests.
Could you ask your question on the [forum](https://discusss.huggingface.co) instead?

Thanks!