noamaanMulla-03/transformers-issues huggingface dataset:
[tokenizers] Fixing #8001 - Adding tests on tokenizers serialization
Author: thomwolf
Pull request: yes

# What does this PR do?

Fixes #8001

Now the tokenizers classes have to send all the keyword arguments of the `__init__` up to the base class of the tokenizer (by `super().__init__`) were they are stored in `init_kwargs` for serialized saving/reloading with `save_pretrained/from_pretrained`.

Adding a test on tokenizers serialization that all the keyword arguments of the `__init__` are found in the saved `init_kwargs` to avoid forgetting to send some arguments up in future (and current) tokenizers.

Make T5 tokenizer serialization more robust.

## 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 the 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.

 albert, bert, XLM: @LysandreJik
 GPT2: @LysandreJik, @patrickvonplaten
 tokenizers: @mfuntowicz
 Trainer: @sgugger
 Benchmarks: @patrickvonplaten
 Model Cards: @julien-c
 Translation: @sshleifer
 Summarization: @sshleifer
 examples/distillation: @VictorSanh
 nlp datasets: [different repo](https://github.com/huggingface/nlp)
 rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)
 Text Generation: @patrickvonplaten, @TevenLeScao
 Blenderbot, Bart, Marian, Pegasus: @sshleifer
 T5: @patrickvonplaten
 Rag: @patrickvonplaten, @lhoestq
 EncoderDecoder: @patrickvonplaten
 Longformer, Reformer: @patrickvonplaten
 TransfoXL, XLNet: @TevenLeScao, @patrickvonplaten
 examples/seq2seq: @sshleifer
 examples/bert-loses-patience: @JetRunner
 tensorflow: @jplu
 examples/token-classification: @stefan-it
 documentation: @sgugger
 -->


Comments:
do_lower_case not saved/loaded correctly for Tokenizers
Author: tholor
Pull request: no

## Environment info
- `transformers` version: 3.4.0
- Platform: Linux-5.4.0-52-generic-x86_64-with-debian-buster-sid
- Python version: 3.7.6
- PyTorch version (GPU?): 1.5.1 (False)
- Tensorflow version (GPU?): not installed (NA)
- Using GPU in script?: No
- Using distributed or parallel set-up in script?: No

### Who can help
@mfuntowicz

## Information

The `do_lower_case` property of BertTokenizer is not correctly restored after saving / loading.

## To reproduce

```python
from transformers import BertTokenizer

tokenizer = BertTokenizer.from_pretrained("bert-base-cased")
print(tokenizer.do_lower_case)

tokenizer.save_pretrained("debug_tokenizer")
tokenizer_loaded = BertTokenizer.from_pretrained("debug_tokenizer")
print(tokenizer_loaded.do_lower_case)

``` 
returns
```
False
True
``` 
## Expected behavior
Same object attributes after saving / loading 

Comments:
Oh! I'll take a look, thanks for the report @tholor 

Thanks for the fast fix @thomwolf ! Very much appreciated!
Minor typo fixes to the tokenizer summary in the docs
Author: albanie
Pull request: yes

Minor typo fixes to the tokenizer summary in the docs

# What does this PR do?

Minor typo fixes to the tokenizer summary in the docs

## Before submitting
- [x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).

## Who can review?
 documentation: @sgugger



Comments:
[Seq2Seq] Allow EncoderDecoderModels to be trained with Seq2Seq
Author: patrickvonplaten
Pull request: yes

# What does this PR do?

This PR changes the Seq2Seq Trainer a bit to:
1) Make it work with `EncoderDecoder`
2) Align its API more with the general `Trainer`

@sshleifer @patil-suraj @sgugger - it would be great if you could take a look and give your general opinion on it :-) 
If this would be ok for you, I will fix the examples test.

Comments:
LGTM, thanks for aligning it! We just need some way to pass `eval_beams` and `max_gen_length`.

>We can have a `Seq2SeqTrainingArguments` that subclasses `TrainingArguments` if that helps.

@sgugger  we do have `Seq2SeqTrainingArguments` class
https://github.com/huggingface/transformers/blob/d99ed7ad618037ae878f0758157ed0764bd7f935/examples/seq2seq/finetune_trainer.py#L37

> @sgugger we do have `Seq2SeqTrainingArguments` class

Ah, had forgotten about that :-)

`eval_beams`/`eval_max_gen_length` reasoning:
@patil-suraj said exactly this LOL, but in my words:
users are not good at modifying configs locally. We want to have a way to run `num_beams=2` during the generation step, but then end up with a trained model with the default # beams. In general, we try not to manipulate config attributes that would only be desired during training.


Also would <3 an encoder decoder test in `examples/seq2seq/test_finetune_trainer.py`. 


After discussion @sshleifer - changed the `Seq2SeqTrainer` to be fully backwards compatible and to work with EncoderDecoder.
@sshleifer - cannot add EncDec test yet because the complete command line setup is too constrained (requires `prepare_seq2seq_batch` to be defined for all tokenizers, etc...) => will see how to add this in the future. 

@sshleifer , @patil-suraj - could you do another review please? :-) 

Should be good - I don't really see how -100 would slow down the TPU, but let's wait for @LysandreJik opinion here. 

Can't seem to reply to the comment, but yes, the line @sshleifer is pointing at will slow down on TPU since it's probably using a `torch.where` behind the scene which does not have an XLA operation AFAIK.

> Can't seem to reply to the comment, but yes, the line @sshleifer is pointing at will slow down on TPU since it's probably using a `torch.where` behind the scene which does not have an XLA operation AFAIK.

Okey, I see -> let's move back in the old CE loss function then to keep backward compatibility! 

@sshleifer - one last review please :-)
DataCollatorIntegrationTest::test_nsp failing on GPU
Author: sshleifer
Pull request: no

@sgugger I believe you are the right person to tag?

```python
=================================== FAILURES ===================================
_____________________ DataCollatorIntegrationTest.test_nsp _____________________
[gw0] linux -- Python 3.7.6 /home/hf/actions-runner_transformers/_work/transformers/transformers/.env/bin/python

self = <tests.test_data_collator.DataCollatorIntegrationTest testMethod=test_nsp>

    @slow
    def test_nsp(self):
        tokenizer = AutoTokenizer.from_pretrained("bert-base-cased")
        data_collator = DataCollatorForNextSentencePrediction(tokenizer)
    
        dataset = TextDatasetForNextSentencePrediction(tokenizer, file_path=PATH_SAMPLE_TEXT, block_size=512)
        examples = [dataset[i] for i in range(len(dataset))]
        batch = data_collator(examples)
        self.assertIsInstance(batch, dict)
    
        # Since there are randomly generated false samples, the total number of samples is not fixed.
        total_samples = batch["input_ids"].shape[0]
        self.assertEqual(batch["input_ids"].shape, torch.Size((total_samples, 512)))
        self.assertEqual(batch["token_type_ids"].shape, torch.Size((total_samples, 512)))
>       self.assertEqual(batch["masked_lm_labels"].shape, torch.Size((total_samples, 512)))
E       KeyError: 'masked_lm_labels'
```

https://github.com/huggingface/transformers/runs/1303479917?check_suite_focus=true

Comments:
Think that should just be renamed `"labels"`. Will look tomorrow and fix.
minor model card description updates
Author: joeddav
Pull request: yes
Labels: name: model card

# What does this PR do?

Makes a few minor updates to the [joeddav/xlm-roberta-large-xnli](https://huggingface.co/joeddav/xlm-roberta-large-xnli) model card, such as removing the reference to the deprecated zero shot demo when the user can play with zero shot classification via the embedded widget. Also links to distilled bart models.

Comments:
PPL guide code snippet minor fix
Author: joeddav
Pull request: yes

# What does this PR do?

Minor fix to the code snippet in the [perplexity guide](https://huggingface.co/transformers/perplexity.html), as discussed in [this thread](https://discuss.huggingface.co/t/guide-the-best-way-to-calculate-the-perplexity-of-fixed-length-models/193).

Previously the snippet didn't take into account the length of the the last loop over the data, which can be shorter than the specified `stride` length.

Comments:
Minor error fix of 'bart-large-cnn' details in the pretrained_models doc
Author: forest1988
Pull request: yes

I found that there seemed to be a mistake regarding "facebook/bart-large-cnn" in the pretrained_models doc.

# What does this PR do?

While I check the model explanations in the pretrained_models doc, I found that there seemed to be a mistake.

Regarding `facebook/bart-large-cnn`, the details of the model is as follows:

```
12-layer, 1024-hidden, 16-heads, 406M parameters (same as base)
bart-large base architecture finetuned on cnn summarization task
```

If my understanding is correct, it seems that `12-layer` and `(same as base)` should be `24-layer` and `(same as large)`.

I asked a question in the forum about this:
https://discuss.huggingface.co/t/there-seems-to-be-a-mistake-in-documentation-pretrained-models-html-regarding-bart/1746/

## Before submitting
- [x] 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 the it if that's the case.
- [x] 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?

documentation: @sgugger
(Thank you for kindly answering my question in the forum!)


Comments:
Thanks!

Thank you, too!
invalid argument wwm passed to the run_language_modeling.py file
Author: MohammadrezaBanaei
Pull request: yes



# What does this PR do?
--wwm cant be used as an argument given run_language_modeling.py and should be changed to --whole_word_mask
<!--
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 -->




## Before submitting
- [x ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).

@stefan-it

Comments:
BlenderbotSmallTokenizer throws tuple index out of range error for stopword
Author: Sumegh-git
Pull request: no

Using transformers==3.4.0

Script used:
```

from transformers import BlenderbotSmallTokenizer, BlenderbotForConditionalGeneration
mname = 'facebook/blenderbot-90M'
tokenizer = BlenderbotSmallTokenizer.from_pretrained(mname)

sentence = "."
tokenizer(sentence)['input_ids']
```

This throws `IndexError: tuple index out of range`

Comments:
Fix + Test
Author: LysandreJik
Pull request: yes

Fix an edge case of the blenderbot-90 tokenizer.

Closes #8029 

# Context

If the blenderbot-90 tokenizer is used to tokenize the following sequence:
```py
sequence = "Ok ."
```
It will split it in two tokens at first:
https://github.com/huggingface/transformers/blob/8bbe8247f13057b7df1b2c9abbfacb05b30020bf/src/transformers/tokenization_blenderbot.py#L221

Those two tokens will be `['Ok', '.']`

The issue is that, when passed the second token, the `bpe` method will convert it from `'.'` to `' .'` here:
https://github.com/huggingface/transformers/blob/8bbe8247f13057b7df1b2c9abbfacb05b30020bf/src/transformers/tokenization_blenderbot.py#L160

This then gets split on spaces here:
https://github.com/huggingface/transformers/blob/8bbe8247f13057b7df1b2c9abbfacb05b30020bf/src/transformers/tokenization_blenderbot.py#L166

This is where the issue lies, as it creates two strings: `["", "."]`, the first one being empty.

It then crashes a bit further as we try to index the empty string:
https://github.com/huggingface/transformers/blob/8bbe8247f13057b7df1b2c9abbfacb05b30020bf/src/transformers/tokenization_blenderbot.py#L171

## Proposal

Ensure that the token has a length > 0 before trying to manage it, otherwise ignore that token.

Added a test.




Comments:
[docs] [testing] distributed training
Author: stas00
Pull request: yes

We figured out how to support distributed training with `pytest`, this is a preliminary doc snippet to help those in need to find the current implementation. I'm sure it will evolve as we  have more tests with varying needs, but for now that's all we have.

@sgugger 

Comments:
One followup is to update the `test_trainer_distributed` to work with pytest. Then ideally, if we could have one command to run all those tests, that would be awesome (maybe we can use a pytest marker to mark all distributed-specific tests so it's easy to select them all?)

I will port `test_trainer_distributed` - thank you for flagging that, @sgugger 
Tracking it here: https://github.com/huggingface/transformers/issues/8058
Access bert output with output_hidden_states=True  of TFBertForSequenceClassification fails
Author: datistiquo
Pull request: no

Hey,

I want to access the output of the main bert model inside the TFBertForSequenceClassification model with output_hidden_states :

`bert_model = TFBertForSequenceClassification.from_pretrained('bert-base-german-cased', output_hidden_states=True)
`
then 

```
print(bert_model.summary())
print(bert_model.get_layer("bert").output)
print(bert_model.layers[0].output[2]) ->yields error
```

bert_model.get_layer("bert").output gives just the 2 outputs for last_hidden_state and pooled_output but the hiden_states are missing.

Why are the hidden_sates not available though I set output_hidden_states=True ?


Comments:
Hello! Could you provide your software versions so that I may investigate?

I get an error earlier: `print(bert_model.get_layer("bert").output)`:
```
Traceback (most recent call last):
  File "<input>", line 4, in <module>
  File "/home/lysandre/Workspaces/Python/transformers/.env/lib/python3.6/site-packages/tensorflow/python/keras/engine/base_layer.py", line 2105, in output
    raise AttributeError('Layer ' + self.name + ' has no inbound nodes.')
AttributeError: Layer bert has no inbound nodes.
```

Actually I found weird things, because previously it worked, but after a windows update I think my version of the transformer librry was set back somehow. Because I used version 3.0.2, and there there was no output at all of the hidden states. But now for the latest stable version (3.4) it works. 

Glad to hear it!
wrong decoder_input_ids[:,0] for MarianMT models ?
Author: sweta20
Pull request: no
Labels: name: marian

## Environment info
<!-- 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! -->
     
- `transformers` version: 3.3.1
- Platform: Linux
- Python version: 3.7.0
- PyTorch version (GPU?): 1.6.0
- Using GPU in script?: Yes

### Who can help: @sshleifer 

## Information

Model I am using (Bert, XLNet ...): MarianMTModel

The problem arises when using:
* [x] the official example scripts: (give details below)

```from transformers import MarianTokenizer
tok = MarianTokenizer.from_pretrained('Helsinki-NLP/opus-mt-en-de')
src_texts = [ "I am a small frog.", "Tom asked his teacher for advice."]
tgt_texts = ["Ich bin ein kleiner Frosch.", "Tom bat seinen Lehrer um Rat."]  # optional
batch_enc: BatchEncoding = tok.prepare_seq2seq_batch(src_texts, tgt_texts=tgt_texts)
# model(**batch) should work
```

model(**batch)  doesn't work as intended because [shift_tokens_right](https://github.com/huggingface/transformers/blob/03ec02a667d5ed3075ea65b9f89ef7135e97f6b4/src/transformers/modeling_bart.py#L226) adds eos token to generate the target sequence.

```
shift_tokens_right(batch["labels"], model.config.pad_token_id)
```
returns
```
[[0,   105,   495,    53,  5324, 17279,   649,     3],
 [0,  2136,  8818,   715,  5832,    91,   688,     3]]
```
instead of
```
[[58100,   105,   495,    53,  5324, 17279,   649,     3],
 [58100,  2136,  8818,   715,  5832,    91,   688,     3]]
```
Here, "58100" is the decoder_start_token_id.

Comments:
This is the intended behavior.
It is very counterintuitive to me as well, but changing `prepare_seq2seq_batch` or `shift_tokens_right` to end up with `decoder_start_token_id` at the 0th position of `decoder_input_ids` seems to lead to worse fine-tuning performance.

If you have evidence to the contrary, I would be happy to change it.

I'm running the experiment now on wmt-en-ro, we'll see how it goes!

Thanks! I was using the pre-trained model for scoring (src, tgt) pairs and didn't actually get a chance to check the impact on finetuning yet. 

I ran this last night, and finetuning loss was identical with and without the change.
Bleu was within 0.1 (`master` was slightly higher).
Here is the branch if you want to play with it: https://github.com/sshleifer/transformers_fork/tree/hack-batches-v2
[testing] fixing crash in deberta
Author: stas00
Pull request: yes

This PR fixes a crash in deberta tests w/ pytorch-1.7+:
```
RuntimeError: diff_view_meta->output_nr_ == 0 INTERNAL ASSERT FAILED at "/opt/conda/conda- \ 
bld/pytorch_1603436966316/work/torch/csrc/autograd/variable.cpp":363, please report a bug to PyTorch.
```

All credits go to @gchanan, thank you! For details of why please see https://github.com/huggingface/transformers/issues/8022#issuecomment-716252599

Fixes: #8022

Comments:
p.s. apparently there has been a deprecation warning with pytorch-1.6, but since this project isn't quite paying attention to the warnings it lead to this crash with pytorch-1.7. It seems to be a weird situation where a deprecation warning hasn't been turned into an error and instead leading to a crash with this particular issue, but perhaps setting an intention to keep the warnings in check would save a possible hassle in the future. https://github.com/huggingface/transformers/issues/8060

Perhaps what would help is to automatically turn selective types of warnings to errors, and thus not let those slide until they become a problem.

Also having a scheduled CI that runs occasionally on pytorch-nightly (and any release candidates) would give an early alert.
Ignore me!
Author: davidefiocco
Pull request: yes

I did this PR by mistake, and found out late. Sorry Hugging Face!

Comments:
Sorry for the mess-up, was working on a fork of mine (facepalm) and accidentally PR to the repo.

No worries – but potentially open to a PR adding Azure logging if it makes sense on your end!

Hi @julien-c , thanks for pointing that out!

I am running `transformers` on AzureML (finetuning on classification tasks mainly), as I find it convenient because of the e.g. automatic compute provisioning and downsizing, experiment tracking, model versioning capabilities Azure offers.

I don't know what's the best way to contribute with respect to this (i.e. sharing code running finetuning of BERT using AzureML), but here's what I know:

- Implementing tensorboard-like capabilities in AzureML experiments can be done with trivial modifications to the current `trainer.py`, along the lines of https://github.com/huggingface/transformers/commit/939f6798acb178e3d8388e4597ef7e3dfcef0bf2#diff-cd3b4dd7d09c0b32ff40ccc1840a4da4

- Those trivial changes allow to see in the AzureML frontend trends like

![image](https://user-images.githubusercontent.com/4547987/93239437-0ed77880-f783-11ea-803e-f674a378f611.png)

which can be handy when running experiments.

- To launch one experiment, one would use Azure-specific code like that in https://github.com/microsoft/AzureML-BERT/blob/master/finetune/PyTorch/notebooks/Pretrained-BERT-GLUE.ipynb

Microsoft had started a pretty cool repo at https://github.com/microsoft/AzureML-BERT/ with solutions for this using https://github.com/huggingface/transformers/tree/v0.6.2 . What I think is happening though is that `transformers` has evolved quite a lot since then and they would need to refresh those examples.
 
I honestly don't know if it'd make sense to PR https://github.com/microsoft/AzureML-BERT (and not clutter the `transformers` with platform-specific code). WDYT? Cheers!


The idea above is resurrected here https://github.com/huggingface/transformers/pull/8062