noamaanMulla-03/transformers-issues huggingface dataset:
What's the correct way to use add_prefix_space for the fast RoBERTa tokenizer in 3.0.0/3.0.1?
Author: ZhaofengWu
Pull request: no
Labels: name: Core: Tokenization

The documentation says we should pass this flag to **the encoding methods**.

https://github.com/huggingface/transformers/blob/58cca47c16149e43d1b516623d59e3c5d97f695e/src/transformers/tokenization_roberta.py#L259-L260

However, the encoding methods don't take such an argument, and passing it in actually causes an error:

```
File "test.py", line 127, in func
  self.tokenizer.encode(token, add_special_tokens=False, add_prefix_space=True)
File ".../transformers/tokenization_utils_base.py", line 1425, in encode
  **kwargs,
File ".../transformers/tokenization_utils_base.py", line 1737, in encode_plus
  **kwargs,
File ".../transformers/tokenization_gpt2.py", line 377, in _encode_plus
  return super()._encode_plus(*args, **kwargs)
File ".../transformers/tokenization_utils_fast.py", line 420, in _encode_plus
  **kwargs,
File ".../transformers/tokenization_gpt2.py", line 367, in _batch_encode_plus
  return super()._batch_encode_plus(*args, **kwargs)
File ".../transformers/tokenization_utils_fast.py", line 313, in _batch_encode_plus
  raise ValueError(f"Keyword arguments {kwargs} not recognized.")
ValueError: Keyword arguments {'add_prefix_space': True} not recognized.
```

Then, I assumed that this documentation was a typo and we should pass this argument to `__init__`. But why is it default to `False` in `__init__`? Don't we pretty much always need to add a prefix space? Does `from_pretrained` set it to `True` automatically? If not, should we always do `AutoTokenizer.from_pretrained(..., use_prefix_space=True)`?

https://github.com/huggingface/transformers/blob/58cca47c16149e43d1b516623d59e3c5d97f695e/src/transformers/tokenization_roberta.py#L299-L314

Comments:
Thanks for flagging, the docstrings were indeed very confusing!
I hope the new doc introduced in #5559 is clearer and solves your problem. Please reopen if needed.

Thanks for the clarification in the new doc! Does it mean RoBERTa was pre-trained **without** this add_prefix_space behavior? In other words, in the original model, does it encode "Hello world" with 2 or 3 tokens, excluding special tokens? If it was pre-trained without adding the prefix space, does it mean the **fast tokenizers** in the pre-3.0.0 transformers versions were adding this prefix space incorrectly?

E.g. in 2.11.0
```
>>> from transformers import RobertaTokenizerFast
>>> tokenizer = RobertaTokenizerFast.from_pretrained("roberta-base")
>>> tokenizer.tokenize("Hello world")
['ĠHello', 'Ġworld']
```

I think it was incorrect, yes, since it currently returns `['Hello', 'Ġworld']` like the slow tokenizer. Multiple bugs in fast tokenizer were fixed in 3.0.0.
[Seq2SeqTrainer] Move import to init to make file self-contained
Author: patrickvonplaten
Pull request: yes

# What does this PR do?

Seq2SeqTrainer can be used as an independent file if no `label_smoothing` is done. This PR move the import to the init to make it possible to simple download this file and use it as it is without any dependencies for standard seq2seq training.

<!--
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 -->

Would be awesome if @patil-suraj  and @stas00 could review as well :-) 



Comments:
I'm okay with this, but users will still need to copy paste or rewrite  `Seq2SeqTrainingArguments`   and pass those as `args` instead of default `TrainingArguments`, since we assume `args` is `Seq2SeqTrainingArguments` in multiple methods
Updated Reformer to use caching during generation
Author: guillaume-be
Pull request: yes

# What does this PR do?

The current reformer implementation supports caching of buckets and states, but this is not used during generation. Running a generation example in debugging mode, such as 
```python
from transformers import ReformerModelWithLMHead, ReformerTokenizer

model = ReformerModelWithLMHead.from_pretrained("google/reformer-crime-and-punishment").cuda()
tok = ReformerTokenizer.from_pretrained("google/reformer-crime-and-punishment")
output = tok.decode(
    model.generate(tok.encode("Notwithstanding", return_tensors="pt").cuda(),
                   do_sample=True,
                   temperature=0.7,
                   max_length=100,
                   use_cache=True)[0])
```
One can see that the `past_buckets_states` passed to the attention are always `None` (at https://github.com/huggingface/transformers/blob/504ff7bb1234991eb07595c123b264a8a1064bd3/src/transformers/modeling_reformer.py#L365)

This is because the name of the past states for the reformer are neither `past_key_values` or `mems`.
This PR adds the name of the past states to the generation `past` allocation.

Generally, it may make sense to harmonize the `past` value for all models, so that the `generate` function generalizes better 

## Who can review?

Text Generation: @patrickvonplaten, @TevenLeScao
Reformer: @patrickvonplaten


Comments:
Great catch!

Let's merge that quickly so that I can integrate it into https://github.com/huggingface/transformers/pull/6949/files#diff-b7601d397d5d60326ce61a9c91beaa2afa026014141052b32b07e1d044fbbe17

Actually, we would have to add in two spots of this `generate` version. Considering that we will merge the big generate refactor today, I just added your fix quickly here: https://github.com/huggingface/transformers/pull/6949/commits/12b54eceeb57229ffd940cadf47e6e159b101d8e

Mentioned your PR at the fix - hope it's ok for you to close this PR to avoid any more merge conflicts.

Thanks a lot!
Create README.md
Author: Jiaxin-Pei
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 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:
Model upload and sharing - delete, update, rename....
Author: miki537
Pull request: no

## 🚀 Feature

It would be great to have the option of deleting, renaming and adding description to the community models. I saw that there are already some errors in the model names which can probably not be fixed because of this missing functionality. 

We should have something like:
transformers-cli delete
transformers-cli rename
transformers-cli update

## Motivation

I think that without this options the list will get quite messy in the future. Also as mentioned already in other issues(#2281 and #2520) we should be able to add how we trained the model, what dataset was used etc.



Comments:
Hi @miki537, we already have `transformers-cli s3 rm ____` but it is not super well documented.

I'll improve the documentation on that point. Also `transformers-cli upload` will overwrite existing files with the same name so you can already update files.

S3 doesn't not support moving/renaming files so I'm reluctant to introduce a `rename` (which would need to download the files locally then re-upload with new name – this is what the official aws-cli does)

I'll close this for now, feel free to reopen if it's not well documented enough (or even better, improve it and create a PR :)

Hi Julien, I'm also trying to delete some of my shared models, however, transformers-cli s3 rm ____ seems not working for me. All the models are still there after the command is done

Which model(s) do you intend to delete @Jiaxin-Pei?

@julien-c 
Here are they:
pedropei/question-intimacy-DEMO
pedropei/question-intimacy-demo
pedropei/random-demo



done
Updated ConversationalPipeline to work with encoder-decoder models
Author: guillaume-be
Pull request: yes

# What does this PR do?

This PR extends the capabilities of the existing ConversationalPipeline to work with Encoder Decoder models (such as BlenderBot). 

The pipeline has been modified as follows:
- history generated from concatenation of the inputs to the generated tokens for encoder-decoder (decoders use directly the generated tokens that contain the initial prompt)
- updated the cut-off position for generated tokens (1 for encoder-decoders, `input_length` for decoders)
- updated the clean-up script to remove all `pad_tokens` if `pad_token` != `eos_token`. Otherwise, remove pad_tokens from the second found if `pad_token` and `eos_token` are identical (previous behaviour of the pipeline). This is needed otherwise the models with a specific `eos_token` will keep an unnecessary `pad_token` that affects generation quality for subsequent rounds.

This has been tested with the BlenderBot 90M model (requires https://github.com/huggingface/transformers/pull/8205), producing the following output: https://gist.github.com/guillaume-be/380b98ec1ef91d0f6e3add5914dd92ce

## Who can review?

 Text Generation: @patrickvonplaten, @TevenLeScao
 Blenderbot, Bart, Marian, Pegasus: @sshleifer


Comments:
> Looks great! Can we maybe add one test with the small `BlenderbotModel` to `/home/patrick/hugging_face/transformers/tests/test_pipelines_conversational.py` ?

Thank you @patrickvonplaten , I added an integration test using BlenderBot 90M.

Merging, unrelated failure.
[examples] minimal version requirement run-time check in PL
Author: stas00
Pull request: yes

This PR adds a run-time version check for PL. Via the warning for now. This is a follow up to https://github.com/huggingface/transformers/pull/7852#issuecomment-718144095

In the nature of development we don't constantly re-run `pip install -r requirements.txt` so often when a breaking change is introduced we have to signal to each other - hey, upgrade your PL or so. It'd be much simpler to let the program do this automatically for us.

for now one needs to update requirements.txt and the relevant .py files, but we could automate this to have one source to maintain - parse `requirements.txt` and pull the important min-version from there...

for now this is just a hardcoded plain check.

**My only suggestion is to make it an error** - there are too too many warnings in the test suite for someone to notice this yet another one - so I vote for making it an error.

@sshleifer, @sgugger, @LysandreJik 

Comments:
[Seq2Seq] Make Seq2SeqArguments an independent file
Author: patrickvonplaten
Pull request: yes

# What does this PR do?

By putting all `Seq2SeqTrainingArguments` logic in a separate file, the `Seq2SeqTrainer` and `Seq2SeqTrainingArguments` can be used as standalone files without having to download any additional files because of other dependencies. 


Comments:
Clean Trainer tests and datasets dep
Author: sgugger
Pull request: yes

# What does this PR do?

This PR removes the installation of datasets from master and uses the dependency already in `testing` instead. It also cleans up a bit the tests in Trainer by:
- using the decorator `requires_datasets` when needed
- using a temp dir for the output of one test, to avoid some files to be created when the user has optuna installed

Comments:
[CIs] Better reports everywhere
Author: stas00
Pull request: yes

Continuing the work in https://github.com/huggingface/transformers/pull/8110 and https://github.com/huggingface/transformers/pull/8163 this PR does the following:

* [x] rename `pytest --make_reports` to `pytest --make-reports` for consistency with the rest of `pytest` opts that don't use `_`
* [x] move the `--make_reports` opt adding to a shared location and load it only once to avoid `pytest` failure 
   - some pytest plugins like `pytest-instafail` load `tests/conftest.py` even when running `examples`
   - now we can run tests from both test suites at once
* [x] rename `reports/report_foo` to `reports/foo` - avoid repetition
* [x] install `--make_reports` in all CIs: circleci and github actions 
* [x] make the reports available via artifacts
* [x] always cat short failure reports for github actions in its own "tab" since getting to artifacts there is a cumbersome process. I'm not sure this is needed in circleci jobs since each report in artifacts is available in 2 clicks, so I left the `cat *failures_short.txt` out on CircleCI jobs.
* [x] fixed a few issues in the github actions job configuration

@sgugger, @LysandreJik, @sshleifer 


Comments:
And and update on proposing this multiple report files feature to be a core feature in pytest https://github.com/pytest-dev/pytest/issues/7972 - one of the developers vetoed it, so I guess it will remain just here for now. It makes no sense for this not to be a core feature of pytest, as we are just splitting the huge mess of everything being dumped to the terminal to just one file per report, but there was no invitation to discuss that - just NO. If someone wants to make it into a pytest plugin it'd surely be useful to others.

> I saw the discussion (or lack thereof) on pytest. Their loss! We don't mind having the post-processing in transformers.

Eventually we will either have to port it to pytest hooks or keep up with the pytest API changes, since currently the code uses `pytest` internals and could break should they change those. It's just so much simpler doing that than reinventing the wheel.
Finetuning T5 on translation wmt19(de-en)
Author: rabeehkarimimahabadi
Pull request: no

Dear huggingface team, 
I'd like to train from scratch T5 on wmt19 (de-en), and I see these instructions in your page:
you provided the script for  finetune mbart_cc25, could I just change the model path and it works out of the box for training T5 on a translation task?
I also gets these errors running the provided script, thank you for your help.
Best
Rabeeh

```
(test) rabeeh@brain1:~/ruse/hf/transformers/examples/seq2seq$ ./train_mbart_cc25_enro.sh --output_dir enro_finetune_baseline --label_smoothing 0.1 --fp16_opt_level=O1 --logger_name wandb --sortish_sampler
2020-11-04 01:41:53.720772: W tensorflow/stream_executor/platform/default/dso_loader.cc:59] Could not load dynamic library 'libcudart.so.10.1'; dlerror: libcudart.so.10.1: cannot open shared object file: No such file or directory; LD_LIBRARY_PATH: /usr/local/cuda/lib64:/usr/local/nccl2/lib:/usr/local/cuda/extras/CUPTI/lib64
2020-11-04 01:41:53.720823: I tensorflow/stream_executor/cuda/cudart_stub.cc:29] Ignore above cudart dlerror if you do not have a GPU set up on your machine.
Traceback (most recent call last):
  File "finetune.py", line 442, in <module>
    main(args)
  File "finetune.py", line 383, in main
    model: SummarizationModule = TranslationModule(args)
  File "finetune.py", line 367, in __init__
    super().__init__(hparams, **kwargs)
  File "finetune.py", line 57, in __init__
    if hparams.sortish_sampler and hparams.gpus > 1:
TypeError: '>' not supported between instances of 'function' and 'int'

```

Comments:
Encoder Decoder Model
Author: arditobryan
Pull request: no

Hi, 

I am following the instructions written on the HuggingFace website to use an encoder-decoder model:

from transformers import EncoderDecoderModel, BertTokenizer
import torch

```
tokenizer = BertTokenizer.from_pretrained('bert-base-uncased')
model = EncoderDecoderModel.from_encoder_decoder_pretrained('bert-base-uncased', 'bert-base-uncased') # initialize Bert2Bert from pre-trained checkpoints

#model.save_pretrained('/content/drive/My Drive/NLP/'+'model_1')

# forward
input_ids = torch.tensor(tokenizer.encode("Hello, my dog is cute", add_special_tokens=True)).unsqueeze(0)  # Batch size 1
outputs = model(input_ids=input_ids, decoder_input_ids=input_ids)

# training
outputs = model(input_ids=input_ids, decoder_input_ids=input_ids, labels=input_ids, return_dict=True)
#print(type(outputs)) #Seq2SeqLMOutput
loss, logits = outputs.loss, outputs.logits

# save and load from pretrained
#model.save_pretrained("bert2bert")
#model = EncoderDecoderModel.from_pretrained("bert2bert")

# generation
generated = model.generate(input_ids, decoder_start_token_id=model.config.decoder.pad_token_id)

generated
tensor([[   0, 1012, 1010, 1010, 1010, 1010, 1010, 1010, 1010, 1010, 1010, 1010,
         1010, 1010, 1010, 1010, 1010, 1010, 1010, 1010]])
```

However, I have no idea how to decode the generated output, can anybody pls help?
Thank you

Comments:
Maybe I found out, is it:

```
for i, sample_output in enumerate(generated):
  print("{}: {}".format(i, tokenizer.decode(sample_output, skip_special_tokens=True)))
```
?

You can also make use of `tokenizer.batch_decode(...)`
Translation finetuning error : TypeError: '>' not supported between instances of 'function' and 'int'
Author: rabeehkarimimahabadi
Pull request: no

Dear huggingface team, 
I'd like to train from scratch T5 on wmt19 (de-en), and I see these instructions in your page:
- you provided the script for  finetune mbart_cc25, could I just change the model path and it works out of the box for training T5 on a translation task? any changes needed?
- when you use sortish sampler (line 256 finetune.py) you check the number of gpus, in case using tpus, shall I check the number of cores of tpus for distributed version of dataloader in line 256? 
- does distributed tpu training works for seq2seq model? I wonder why the dataloader is not modified for tpu cores, is this by purpose and works fine for tpus too? 
- I also gets these errors running the provided script, thank you for your help.
Best
Rabeeh

```
(test) rabeeh@brain1:~/ruse/hf/transformers/examples/seq2seq$ ./train_mbart_cc25_enro.sh --output_dir enro_finetune_baseline --label_smoothing 0.1 --fp16_opt_level=O1 --logger_name wandb --sortish_sampler
2020-11-04 01:41:53.720772: W tensorflow/stream_executor/platform/default/dso_loader.cc:59] Could not load dynamic library 'libcudart.so.10.1'; dlerror: libcudart.so.10.1: cannot open shared object file: No such file or directory; LD_LIBRARY_PATH: /usr/local/cuda/lib64:/usr/local/nccl2/lib:/usr/local/cuda/extras/CUPTI/lib64
2020-11-04 01:41:53.720823: I tensorflow/stream_executor/cuda/cudart_stub.cc:29] Ignore above cudart dlerror if you do not have a GPU set up on your machine.
Traceback (most recent call last):
  File "finetune.py", line 442, in <module>
    main(args)
  File "finetune.py", line 383, in main
    model: SummarizationModule = TranslationModule(args)
  File "finetune.py", line 367, in __init__
    super().__init__(hparams, **kwargs)
  File "finetune.py", line 57, in __init__
    if hparams.sortish_sampler and hparams.gpus > 1:
TypeError: '>' not supported between instances of 'function' and 'int'

```

Comments:
The issue solved with setting --gpus 1 explicitly. thanks.
Fix TPU Convergence bug introduced by PR#6151
Author: jysohn23
Pull request: yes

Currently with the bug introduced we're taking two optimizer steps per
batch: one global one, where `xm.optimizer_step` injects a CRS between
all cores in training, and one without. This has been affecting training
accuracy (for example, XLNet GLUE on MNLI is not converging, etc.).

Comments: