noamaanMulla-03/transformers-issues huggingface dataset:
Fixing flaky conversational test + flag it as a pipeline test.
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 -->


## 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).
- [x] Did you write any new necessary tests?


## Who can review?

@patrickvonplaten

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:
[docs] expand install instructions
Author: stas00
Pull request: yes

This PR: expands the "install from source" section in the instruction file to:

  - clarify that the user is not installing the release version but bleed edge
  - expand how to update it
  - gives the shortcut at doing it all in one command and not needing to keep the checkout folder around

@LysandreJik, @sgugger  

Comments:
Pin memory in Trainer by default
Author: abhishekkrthakur
Pull request: yes


Comments:
Could we please go through normal PR review approval cycles? Unless I missed something and there was one.

It looks like my comment on slack was missed where I suggested to use a more specific cl arg name. 

I proposed one of:
- dataloader_pin_memory
- dl_pin_memory

But since we already have `dataloader_num_workers`:
```
            num_workers=self.args.dataloader_num_workers,
            pin_memory=self.args.pin_memory,
```
it should probably be `dataloader_pin_memory`

This is important since there are other ways to pin memory in pytorch.

------------------

This is a general comment - not specific to this PR:

We have this ongoing issue wrt cl arg naming, that we name something and later we realize it's not the best name and then we are concerned with changing the name not to break user's code, so let's think deeply about new cl args names before we add them. Thank you!

@stas00 It seems like I missed this message and when I opened this PR in the morning, I didn't see any comments and @sgugger had approved the PR. For a final check, I asked @LysandreJik who gave me the green light.

To avoid this in future, I would request if PR specific comments are made on the PR itself so that author & other reviewers can go through them and make sure that everything is resolved before merging.

Yes, absolutely. I guess it just fell through the cracks.

And let's have PR description, as simple as:

This PR adds `--pin_memory` to trainer DataLoader and it defaults to True.
pin_memory -> dataloader_pin_memory
Author: abhishekkrthakur
Pull request: yes

Ref: https://github.com/huggingface/transformers/pull/9857#issuecomment-769256215

This PR adds a new argument `dataloader_pin_memory` to `TrainingArguments`. You can use this to pin memory in `DataLoader`.

Comments:
Updated with review comments. Please let me know if/when it's okay to merge :) 

Good for me, thanks a lot!

this is much better, thank you for the adjustment, @abhishekkrthakur
Change the model type after fine-tuning?
Author: jwallat
Pull request: no
Labels: name: wontfix

name: Core: Modeling

name: Usage

# ❓ Questions & Help

Is there a way in the transformer library to fine-tune a transformer on one downstream task and change to another model type (i.e. another downstream task architecture)?

An example: 
We train a BertForQuestionAnswering model (with a linear layer for the span prediction in addition to the regular BERT). Once we finished our training, I want to disregard the linear layer on top and use the adjusted weights of the 12 BERT-layers in sentiment analysis (BertForSentenceClassification).

I am aware that this would result in a not initialized linear layer on top the BertForSentenceClassification, but that would be not problem in my case. 

Comments:
Yes, you should be able to just load the finetuned model into another architecture. The weights of overlapping parameter names will be loaded, and the others will be ignored.

Awesome, thanks for the fast response 

Okay,  I came across a followup question: 
So from what I can tell, the pre-trained models come with the weights for the BertForMaskedLM (as this has been used to do the pre-training).
Is there a way to first fine-tune a model on a downstream task (like sentence classification) and than reload it as BertForMaskedLM, reusing the trained LM head?   

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.


@BramVanroy could you provide a code example of how to this? I want to do sequential training on different tasks without having to save and load models from disk, or change layers directly myself. Ideally I would use some head switcher function or simply have `.from_pretrained()` accept model instances.

@timellemeet Probably best to post a new issue or [a forum post](https://discuss.huggingface.co/).
Smdistributed trainer
Author: sgugger
Pull request: yes

# What does this PR do?

This PR adds support for the variant of `torch.distributed` developed by AWS on SageMaker (`smdistributed`). It's been tested to work on the `run_glue` example.

The main steps are:
- to replace all operations from the `torch.distributed` module by their equivalent in `smdistributed.torch.distributed`.
- use their wrapper for the model instead of `DistributedDataParallel`.

Comments:
Very cool integration.🚀  🚀 🔥 🔥 
I'm doing some tests over the day for single-GPU and multi-GPU and let you know if I find something strange
where is position_embedding_type used
Author: awdrgyjilplij
Pull request: no

When I was using pytorch Electra Model, I read its source code but I didn't find where position_embedding_type is used.
So did I miss something?

Comments:
It is used quite a lot! Here for example:

https://github.com/huggingface/transformers/blob/4c3ae89ad3215c3252ebf8ce964795ba8813d810/src/transformers/models/electra/modeling_electra.py#L194-L196

Actually just Ctrl+F "position_embedding_type" in this file and you should be able to find out where it's used :) (11 occurrences)

thanks
Clarify use of unk_token in slow tokenizers' docstrings
Author: ethch18
Pull request: yes

# What does this PR do?

Currently, the docstrings for slow tokenizers' `tokenize()` method claim that unknown tokens will be left in place, in contrast to the fast tokenizers' behavior.  In reality, both convert unknown tokens to `unk_token`.

Fixes #9714 


## 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 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?

@LysandreJik 

Comments:
Finetune_Trainer Question
Author: Ansper11
Pull request: no

Hi, I'm new to HuggingFace. I want to fine-tune a BARTForConditionalGeneration model by finetunr_trainer.py for the translation task on google colab, but I didn't figure out how to use the script to fine-tune the model. Could anyone help me to show a quick example? Thanks

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? You'll get more help there.

The docs regarding the maintained trainer are available [here](https://github.com/huggingface/transformers/tree/master/examples/seq2seq#new-script) and may be useful to you.
Thanks!

Oh sorry about that, Thank you very much
Added do_lower_case parameters for tokenizer in mlm training.
Author: K-Mike
Pull request: yes

Added do_lower_case while training mlm. Useful for training cased BER, for instance. 

Comments:
Hi there, thanks for your PR!
The examples scripts are kept simple and without too much functionality so users can easily understand and tweak them for their needs (they are just examples, they do not mean to cover **everything**). As you saw, it's super easy to add things like this option, if needed. The PR will stay this to demonstrate how, but I don't think we will merge it.

As far as I know this is already done in the Tokenizer logic. You can define this lower casing option in the `tokenizer_config.json` - and this is done for quite a lot models. I dont't see any reason to have this as an extra cli option 🤔 

I just tried to be useful :)

No problem at all. If you want to make sure something you are working on will be accepted, don't hesitate to open an issue about it first, that way we can tell you if it's desirable or not :-)
Be sure to check the [good first issues](https://github.com/huggingface/transformers/issues?q=is%3Aissue+is%3Aopen+label%3A%22Good+First+Issue%22) if you want to try something else!
When on sagemaker use their env variables for saves
Author: sgugger
Pull request: yes

# What does this PR do?

When on SageMaker, the content of the env variable "SM_OUTPUT_DATA_DIR" should be used to save training artifacts (such as our checkpoint) so make it overwrite the `output_dir` (and make that argument optional so it doesn't need to be passed for sagemaker training).
Then the final model will be easy to deploy if it's also saved to the content of the env variable "SM_MODEL_DIR" so adding that as well.


Comments:
[seq2seq] correctly handle mt5
Author: stas00
Pull request: yes

This PR fixes `seq2seq/utils.py` to handle `mt5` like it does `t5`.

Ideally there should be a test, which would require creating a tiny model for mt5, but I'm being told this code is going away anyway, so there is no point investing energy into it.

Fixes: https://github.com/huggingface/transformers/issues/9865

@patil-suraj, @sgugger 

Comments:
> when you port this to the new run_seq2seq, it would be great to try to find a way to make this not use any special code for a given model

I'm working on it in  #9844, it's not finished though. We might need to add `get_input_embeddings` and `get_pos_embeddings` methods to every s2s model, to avoid special cases.

If we need to add some methods to deal with the special cases, I would prefer it (otherwise the script might fail with new seq2seq models).
[trainer] [deepspeed] refactor deepspeed setup devices
Author: stas00
Pull request: yes

Following the discussion at https://github.com/huggingface/transformers/pull/9798#pullrequestreview-578822445 as we now have multiple integrations with complex unique setups, @sgugger and I agreed that it's better to have a small duplication of a few lines of code but to make it much easier to understand what goes on for a specific integration, so rather than further refactoring the recently added sage branch, this PR creates a dedicated branch for DeepSpeed and thus simplifies the general case when straight DDP is used.

There is no functionality change - just a small code reshuffle.

@sgugger 


Comments:
[DOCS] curl links go to 404 not found in NER tutorial
Author: rozeappletree
Pull request: no

Hey, in [NER tutorial](https://huggingface.co/transformers/v2.2.0/examples.html#named-entity-recognition) the curl commands seem outdated

```
curl -L 'https://sites.google.com/site/germeval2014ner/data/NER-de-train.tsv?attredirects=0&d=1' \
| grep -v "^#" | cut -f 2,3 | tr '\t' ' ' > train.txt.tmp
curl -L 'https://sites.google.com/site/germeval2014ner/data/NER-de-dev.tsv?attredirects=0&d=1' \
| grep -v "^#" | cut -f 2,3 | tr '\t' ' ' > dev.txt.tmp
curl -L 'https://sites.google.com/site/germeval2014ner/data/NER-de-test.tsv?attredirects=0&d=1' \
| grep -v "^#" | cut -f 2,3 | tr '\t' ' ' > test.txt.tmp
```

Can you please share new curl requests?

Comments:
the doc link you shared if for V2.2.0, we have updated all the examples in a recent version. You can find the new ner examples here https://github.com/huggingface/transformers/tree/master/examples/token-classification

Thank you @patil-suraj
[doc] nested markup is invalid in rst
Author: stas00
Pull request: yes

Apparently nested markup in RST is invalid: https://docutils.sourceforge.io/FAQ.html#is-nested-inline-markup-possible

So currently this line doesn't get rendered properly, leaving inner markdown unrendered, resulting in:
```
You can create a model repo directly from `the /new page on the website <https://huggingface.co/new>`__.
```

This PR removes the bold markdown which fixes the link.

@sgugger 

Comments:
Improve pytorch examples for fp16
Author: ak314
Pull request: yes

# What does this PR do?

When fp16 is True in pytorch training examples, use padding to multiple of 8, if the currently used data collator allows, to speed up training. If no collator was used, add one using the padding option.
Fixes #9752 


## Who can review?

 Trainer: @sgugger 

Comments:
Thanks for addressing the comments, this is good to go IMO.
Pro-tip, if you edit your description to replace `Issue #9752` by `Fixes #9752`, the issue will be automatically closed when we merge this PR.
run_seq2seq.py : Why we pad labels with -100?
Author: shamanez
Pull request: no

As mentioned in [this line](https://github.com/huggingface/transformers/blob/master/examples/seq2seq/run_seq2seq.py#L437), why we add -100. Can't we just keep pad_token_id ?

Comments:
Please use the [forums](https://discuss.huggingface.co/) for questions like this. We keep issues for bugs or feature requests only.
ImportError: cannot import name 'PreTrainedEncoderDecoder' from 'transformers' (unknown location)
Author: gianfilippo
Pull request: no

Hi,

I am using the library to pretrain my model of choice. I am now interested in setting up an encoder-decoder architecture with my pretrained models, and  the "combiners" seems quite a straightforward way to do that.
Unfortunately, I am getting an import error on both "PreTrainedEncoderDecoder" and "Model2Model"
What am I missing ?
Thanks
Gianfilippo

## 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: 4.3.0.dev0
- Platform: Linux-3.10.0-1062.33.1.el7.x86_64-x86_64-with-glibc2.10
- Python version: 3.8.6
- PyTorch version (GPU?): 1.7.1 (True)
- Tensorflow version (GPU?): 2.4.1 (False)
- Using GPU in script?: NA
- Using distributed or parallel set-up in script?: NA

## To reproduce
Steps to reproduce the behavior:
1.python -c "from transformers import PreTrainedEncoderDecoder"

<!-- If you have code snippets, error messages, stack traces please provide them here as well.
     Important! Use code tags to correctly format your code. See https://help.github.com/en/github/writing-on-github/creating-and-highlighting-code-blocks#syntax-highlighting
     Do not use screenshots, as they are hard to read and (more importantly) don't allow others to copy-and-paste your code.-->
ImportError: cannot import name 'PreTrainedEncoderDecoder' from 'transformers' (unknown location)

## Expected behavior
<!-- A clear and concise description of what you would expect to happen. -->
no error



Comments:
I'm not sure where you have seen those objects: they are nowhere in the transformers library. The library provides  `EncoderDecoderModel`, see the [encoder/decoder doc page](https://huggingface.co/transformers/model_doc/encoderdecoder.html).

Hi, I was reading this  (https://medium.com/huggingface/encoder-decoders-in-transformers-a-hybrid-pre-trained-architecture-for-seq2seq-af4d7bf14bb8). I also found someone reporting some issue while using the same object here
https://github.com/huggingface/transformers/issues/2206.
Perhaps I am looking at some older version ?



This is indeed from an older version (I guess 2 something or even 1 something). 

Thanks. I will look at the EncoderDecoderModel
Fix Trainer and Args to mention AdamW, not Adam.
Author: gchhablani
Pull request: yes

This PR fixed the issue with Docs and labels in Trainer and TrainingArguments Class for AdamW, current version mentions adam in several places.

Fixes #9628 

The Trainer class in  `trainer.py` uses AdamW as the default optimizer. The TrainingArguments class mentions it as Adam in the documentation, which was confusing.

I have also changed variable names to `adamw_beta1`, `adamw_beta2`, `adamw_epsilon` in `trainer.py`.

- [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?

@LysandreJik 

Comments:
Thanks for opening the PR. As it stands this would break every existing script leveraging the parameters defined, so renaming the parameters is probably not the way to go.

@sgugger, your insight on this would be very welcome.



There is no reason to change all the names of the parameters indeed, and it would be a too-heavy breaking change. `AdamW` is not a different optimizer from `Adam`, it's just `Adam` with a different way (some might say the right way) of doing weight decay. I don't think we need to do more than a mention at the beginning of the docstring saying that all mentions of `Adam` are actually about `AdamW`, with a link to the paper.

Hi @LysandreJik @sgugger. Thanks for your comments, I'll be changing the variables back.

I apologize if this is too silly a question, but how can I run and see how the docs look on a browser after the changes?

You should check [this page](https://github.com/huggingface/transformers/tree/master/docs#generating-the-documentation) for all the information on generating/writing the documentation :-)

I have updated it and also added that by default, the weight decay is applied to all layers except bias and LayerNorm weights while training.

@sgugger My code passed only 3 out of 12 checks, I was unable to run CirlceCI properly. Can you point out the reasons why this happened?

We are trying to get support from them to understand why, but the checks on your PR were all cancelled. I couldn't retrigger them from our interface either.

Hi @sgugger 

I believe a possible reason could be that I followed `transformers` on CircleCI. Maybe it performs checks on my fork of transformers and expects to find some "resources" which aren't there.

I'm not sure how CircleCI works, so this is just a wild guess.