noamaanMulla-03/transformers-issues huggingface dataset:
[fsmt test] basic config test with online model + super tiny model
Author: stas00
Pull request: yes

This PR does:

* [x] Seeing the issue of https://github.com/huggingface/transformers/pull/7659 I realized fsmt didn't have a basic non-slow test that loads the tokenizer via online model files. Luckily a totally unrelated examples test caught this issue in that PR, so adding a very simple quick test in the main test suite, so that it runs by the normal CI.
* [x] while I was building this test, I needed a new tiny model, so I refined the script https://github.com/huggingface/transformers/blob/master/scripts/fsmt/fsmt-make-tiny-model.py and made a new one that creates a 50 times smaller model, so we are now at 60KB, instead of 3MB.

@LysandreJik, @sshleifer 


Comments:
Hi Stas, thanks, #7659 will fix this (we will now require at least one example checkpoint for each tokenizer and we test it automatically).

@thomwolf, I'm not certain why you closed this. This is a tokenizer test that is needed - the issue caught in examples was just a flag that there was a missing test in the normal tests.

Actually, not only it's needed, I will have to expand this test to verify that it **doesn't** get the hardcoded default values, but fetches the correct values from `tokenizer_config.json`. And to change the default values to be different from `TINY_FSMT`, since otherwise it won't be testing the right thing. 

Feel free to add it as part of https://github.com/huggingface/transformers/pull/7659 but please make sure you used different from `TINY_FSMT` hardcoded defaults.

I hope this makes sense. 

Hmm, but you copied `TINY_FSMT` in https://github.com/huggingface/transformers/commit/1885ca7f29ffa5373ea848edad8efab02809c268, how will then the tests check it can fetch that data if this is now hardcoded? I am not following this. Do I need to create `TINY_FSMT2` with different values?

I haven't read the new code in depth, but my gut feeling is that the defaults may mask a problem.

Hi stas, I'll let you read the new code and then we can have a look together.

The basic idea is that we now require a full and working checkpoint for the tokenizers to be fully tested in various conditions and the slow vs. fast compared.

The question of testing that tokenizers load and use `tokenizer_config.json` is another question and we should indeed address it in a subsequent PR if it's not addressed already indeed.

That works. 

But please re-open this PR, since we need it anyway. I will add more changes to it after your big PR merge to ensure that the loading of the tokenizer is properly tested.

This PR is complete now.
New run glue script
Author: sgugger
Pull request: yes

# What does this PR do?

This PR cleans up the `run_glue.py` script to use the Datasets library. Along the way it adds a few fixes in Trainer. The script supports all glue tasks as well as custom user tasks (passed along with a training and validation file in csv or json format). It has been tested on the following setups:
- single GPU
- multi-GPU with DataParallel
- multi-GPU with DistributedDataParallel
- TPU

The README has been updated to reflect the changes, there is just one breaking change from before which is that `data_dir` is not an accepted argument anymore (since Datasets will take care of downloading the data files).

Comments:
Should we start thinking about automating the creation of the metadata block for the model's model card?

here for instance we'd already have this info:
```
---
datasets:
- mrpc
metrics:
- f1
finetuned_from: bert-base-cased
---
```

We could think of something like that and add a blank model card to be completed by the user in the final checkpoint. We could also include the results of the last evaluation if there is one.
Only log total_flos at the end of training
Author: sgugger
Pull request: yes

# What does this PR do?

This PR removes the addition of `total_flos` at each (and every) log, since this kind of pollutes them, and only logs it once and for all at the end of training. Users can still define their own callbacks and do more with that value if they really want to, but from what I understood. @TevenLeScao, that value is mainly necessary at the end.

Also, now that it's not in the metrics anymore, I've reverted the default compute metrics to its previous behavior (sum of all metrics) since it's the documented behavior. (cc @madlag) If we want to really change it, we need to put more examples out there.

Comments:
Reload checkpoint
Author: sgugger
Pull request: yes

# What does this PR do?

This PR fixes a few bugs linked to resuming training from a checkpoint, mainly:
- the progress was not properly displayed (beginning at 0 instead of the step from the checkpoint)
- reloading the optimizer state and scheduler state on TPU was causing an error

Tested on TPU, single-GPU and multi-GPU env.

Fixes #4963 
Fixes #7976 


Comments:
RFC: Move `_NoLayerEmbedTokens` to modeling_tf_utils.py
Author: sshleifer
Pull request: no

It has 0 computation and will be needed in many places. Here is the code:


```python
class _NoLayerEmbedTokens:
    """
    this class wraps a the TFSharedEmbeddingTokens layer into a python 'no-keras-layer'
    class to avoid problem with weight restoring. Also it makes sure that the layer is
    called from the correct scope to avoid problem with saving/storing the correct weights
    """

    def __init__(self, layer, abs_scope_name=None):
        self._layer = layer
        self._abs_scope_name = abs_scope_name

    def call(self, inputs, mode="embedding"):
        if self._abs_scope_name is None:
            return self._layer.call(inputs, mode)

        # if an abs scope name is given to the embedding variable, call variable from absolute scope
        with tf.compat.v1.variable_scope(self._abs_scope_name, auxiliary_name_scope=False) as abs_scope_name:
            with tf.name_scope(abs_scope_name.original_name_scope):
                return self._layer.call(inputs, mode)

    def __call__(self, inputs, mode="embedding"):
        if self._abs_scope_name is None:
            return self._layer(inputs, mode)

        # if an abs scope name is given to the embedding variable, call variable from absolute scope
        with tf.compat.v1.variable_scope(self._abs_scope_name, auxiliary_name_scope=False) as abs_scope_name:
            with tf.name_scope(abs_scope_name.original_name_scope):
                return self._layer(inputs, mode)
```


I will copy it into tfbart to avoid getting bogged down in debate, but wdyt @patrickvonplaten ?

Comments:
`TFSharedEmbeddings` is in `modeling_tf_utils.py`
This seems similar.

I'm fine with moving this into `modeling_tf_utils.py`
Move NoLayerEmbedTokens
Author: sshleifer
Pull request: yes

As agreed upon with @patrickvonplaten , this moves the very useful, very model agnostic, `NoLayerEmbedTokens` to `modeling_tf_utils.py`, where it can be used by `TFBart` and `TFT5`.

`TFProphetNet` and other seq2seq may also eventually need.

Comments:
`TFWrappedEmbeddings` is definitely way better, but as a user, I still don't understand what this means. Do you think we could add a comment where it's used? Maybe something along the lines of:

```
# Wraps layer to avoid problems with weight restoring and ensuring we're in the correct TF scope.
```

Done, thanks for writing it out!
[wip] improved github actions workflow
Author: stas00
Pull request: yes

This is a WIP work on https://github.com/huggingface/transformers/issues/7887

Comments:
@sshleifer, I don't think I can do anything here w/o repo permissions. I have no access to the runners.

https://github.com/stas00/transformers/actions/runs/319165633

> No runner matching the specified labels was found: self-hosted, multi-gpu

github is so borked! It now won't let me delete the new workflow - and keeps running it automatically and emailing me on each commit I make elsewhere in this project - what horrors!

ask on https://github.community/, they are pretty responsive.
[gh ci] less output ( --durations=50)
Author: sshleifer
Pull request: yes

Way too much output in [this](https://github.com/huggingface/transformers/pull/7989)
This will make it slightly better.
cc @stas00 


Comments:
Thank you for the heads up - I will be working on all these related issues shortly - too much data indeed, but not just that.

I'd say remove them complete for now and also -rA - I need to experiment and see how to make this data available w/o making logs unusable. 

More context on github actions:


if we can somehow catch the return value of 
bash
```
x= python -m pytest -n 1 --dist=loadfile -s examples --durations=50 | tee test_output.txt
save test_output.txt # always succeeds
sys.exit(x)
```
or something like that, we can make huge progress on the github actions issue and start making artifacts files.

The reason artifacts files broke was that even in the below code, even if line 1 raises an error, line 2 succeeds so github actions thinks the job succeeded
```bash
python -m pytest -n 1 --dist=loadfile -s examples --durations=50 | tee test_output.txt
save test_output.txt # always succeeds
```

oh, unless I'm missing something, we don't need any of the workarounds. 

I already have the first requested component (failures) working, see: https://github.com/huggingface/transformers/pull/7995

Check out the resulting artifacts:
https://app.circleci.com/pipelines/github/huggingface/transformers/14354/workflows/1ccd616e-218f-4ae1-b413-91d2faa0e942/jobs/104363/artifacts

this is what we want right?

`pytest` provides hooks for doing this kind of work, so it's just figuring out which hooks to call.

In your example instead of `x = cmd` what you need to save is `$?` which is the exit status of the command.


That's great, but note that this is all much easier in circleci. My ask is to make it work in github actions.
The failures are already pretty easy to find in circleci.


2) you mean
```bash
x= python -m pytest -n 1 --dist=loadfile -s examples --durations=50 | tee test_output.txt
save test_output.txt # always succeeds
sys.exit($x)
```
?

`test_failures.txt` is really nice!

Ah, good point. let me see what other handy reports I can squeese in circle-ci and then I will move to github actions.

I'm not following your question yet, let me get to github actions and then it'll probably make sense, but yes I'm referring to that example when I said:
> In your example instead of `x = cmd` what you need to save is `$?` which is the exit status of the command.

i.e. `sys.exit($?)` but you must save it right away upon `pytest` completion, since the next command will overwrite it.
Question about loss computing in BartForConditionalGeneration
Author: JamesHujy
Pull request: no
Labels: name: wontfix

I notice that in [BartForConditionalGeneration](https://github.com/huggingface/transformers/blob/master/src/transformers/modeling_bart.py#L1043), the labels and logits are not shifted when computing cross-entropy loss. Should I provide a pre-possessed shifted labels to the model for training?


Comments:
Hi @JamesHujy , yes when training BART you need to shift `labels` and `decoder_input_ids`.

```python3
target_text = "some target text"
enc = tokenizer(target_text , return_tensors="pt")
target_ids = enc["input_ids"]
decoder_input_ids = target_ids[:, :-1].contiguous()
labels = target_ids[:, 1:].clone() 
```

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.
Whole Word Masking Implementation
Author: luffycodes
Pull request: no
Labels: name: wontfix

# 🚀 Feature request

Currently, training the models from scratch like Roberta do not support whole word masking (e.g., language modeling examples). Only pre-trained models are available. Is it possible to include whole word masking in the input layers?

## Motivation

Whole word masking leads to performance boosts. So, adding this feature would be useful if someone wants to train the models from scratch.

Comments:
Would be a great improvement :+1: 

Here's btw. the commit that introduced WWM in BERT:

https://github.com/google-research/bert/commit/0fce551b55caabcfba52c61e18f34b541aef186a

BERT using wordpiece tokenizer, however, roberta uses byte-piece tokenizer. I think the implementations may be slightly different, if not starkly different (due to different start token indicators).

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.
Positional and Segment Embeddings in BERT
Author: AkshitaJha
Pull request: no
Labels: name: wontfix

#### Question

I'm trying to train a BERT language model from scratch using the huggingface library. I'm not sure I completely understand how positional and segment embeddings work in BERT.  The original BERT paper states that unlike transformers, positional and segment embeddings are learned. What exactly does this mean? 

How do positional embeddings help in predicting masked tokens? Is the positional embedding of the masked token predicted along with the word?

How has this been implemented in the huggingface library? 

Comments:
They are not predicted, they are learnt alongside the token embedding themselves. They work exactly the same w.r.t. to training: they are all simply embeddings but with a different vocab size.

https://github.com/huggingface/transformers/blob/9a473f1e43221348334b9e7f95bb45770b7ef268/src/transformers/modeling_bert.py#L154-L156

The output of all three embeddings are summed up before passing them to the transformer layers.

Positional embeddings can help because they basically highlight the position of a word in the sentence. A word in the first position likely has another meaning/function than the last one. Also, the same word likely will have a different syntactic function in the first vs. last position. Positional embeddings thus play some kind of syntactic role where they tell the model that a word can have a different meaning/syntactic function depending on its position.

Thank you for the explanation. What is the benefit of learning positional embeddings as opposed to the technique adopted by transformers where the positional encoding is directly added to each input embedding and not learnt?

When is a positional embedding directly added? An embedding is an embedding and as such always needs to be learned or transfered. 

Section 3.5 of the paper '[Attention is All You Need](https://arxiv.org/pdf/1706.03762.pdf)' explains the positional encoding in the case of transformers. They use _'sine and cosine functions of different frequencies'_ to inject information about the position of the tokens. Learned positional embeddings do not seem to help in the case of the original transformers. BERT on the other hand 'learns' positional embeddings. I wanted to understand the benefits of learning these positional embeddings in BERT.

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.
Loading 'fmikaelian/flaubert-base-uncased-squad' throws unexpected, difficult to comprehend warning
Author: BenoitDalFerro
Pull request: no
Labels: name: wontfix

## Environment info
     
- `transformers` version: 3.0.2
- Platform: Windows 10 Home v2004, Conda 4.8.3, Anaconda CLI 1.7.2
- Python version: 3.7.1
- PyTorch version (GPU?): 1.6.0 CUDA GPU
- Tensorflow version (GPU?): none
- Using GPU in script?: torch.set_default_tensor_type('torch.cuda.FloatTensor')
- Using distributed or parallel set-up in script?: dunno

### Who can help
@fmikaelian @julien-c 

## Information

Model I am using : FlauBERT

The problem arises when using the official example script.

The tasks I am working on is:
* [ ] an official GLUE/SQUaD task: loading 'fmikaelian/flaubert-base-uncased-squad'

## To reproduce

Steps to reproduce the behavior:
```
model_name = 'fmikaelian/flaubert-base-uncased-squad' #flaubert/flaubert_base_cased #flaubert/flaubert_large_cased
config = AutoConfig.from_pretrained(model_name)
tokenizer = AutoTokenizer.from_pretrained(model_name)
model = AutoModelForQuestionAnswering.from_pretrained(model_name, config=config)
```

> Some weights of the model checkpoint at fmikaelian/flaubert-base-uncased-squad were not used when initializing FlaubertForQuestionAnsweringSimple: ['qa_outputs.start_logits.dense.weight', 'qa_outputs.start_logits.dense.bias', 'qa_outputs.end_logits.dense_0.weight', 'qa_outputs.end_logits.dense_0.bias', 'qa_outputs.end_logits.LayerNorm.weight', 'qa_outputs.end_logits.LayerNorm.bias', 'qa_outputs.end_logits.dense_1.weight', 'qa_outputs.end_logits.dense_1.bias', 'qa_outputs.answer_class.dense_0.weight', 'qa_outputs.answer_class.dense_0.bias', 'qa_outputs.answer_class.dense_1.weight']
> - This IS expected if you are initializing FlaubertForQuestionAnsweringSimple from the checkpoint of a model trained on another task or with another architecture (e.g. initializing a BertForSequenceClassification model from a BertForPretraining model).
> - This IS NOT expected if you are initializing FlaubertForQuestionAnsweringSimple from the checkpoint of a model that you expect to be exactly identical (initializing a BertForSequenceClassification model from a BertForSequenceClassification model).
> Some weights of FlaubertForQuestionAnsweringSimple were not initialized from the model checkpoint at fmikaelian/flaubert-base-uncased-squad and are newly initialized: ['qa_outputs.weight', 'qa_outputs.bias']
> You should probably TRAIN this model on a down-stream task to be able to use it for predictions and inference.

## Expected behavior

Not throwing a warning.

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.
Fix BatchEncoding.word_to_tokens for removed tokens
Author: n1t0
Pull request: yes

Fixes https://github.com/huggingface/tokenizers/issues/343

Copied from issue on `tokenizers` repo:

> I'm working with pre-tokenized data (UD-Treebanks) for a sequence-tagging task, since I don't want to inflate the importance of a training example based on the number of word-pieces the token gets split into, I need to map the labels to only the first word-piece of a token.
>
> To achieve this, I was iterating over the words in the original sentence as taken from the treebank and used the word_to_tokens method with the offset of the word in the sentence to get the corresponding token span. If words simply vanish from the sentence, then at first the offsets become invalid and at the final word of the sequence an exception is raised because there's no offset for disappearing words in the sequence. 

This notebook demontrates the issue: 

https://colab.research.google.com/drive/139mVXMQ7jZBBoTpgkribgVpOu6W1u8e9?usp=sharing

~~~Py
import transformers
import torch
tokenizer = transformers.AutoTokenizer.from_pretrained("bert-base-multilingual-cased", use_fast=True)
batch = [["Test", "\xad", "test"]]
encoded_batch = tokenizer.batch_encode_plus(
    batch,
    padding=True,
    is_pretokenized=True,
    return_tensors='pt',
    truncation=True)
first_pieces = torch.zeros_like(encoded_batch.attention_mask, dtype=torch.bool)

for row, sentence in enumerate(batch):
    for col, token in enumerate(sentence):
        idx = encoded_batch.word_to_tokens(row, col)[0] # this method raises the exception
        first_pieces[row, idx] = True
~~~

Comments:
german medbert readme
Author: smanjil
Pull request: yes
Labels: name: model card

# 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.
- [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?

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:
Closed in favor of #8002
Added model cards for Tagalog ELECTRA models
Author: jcblaisecruz02
Pull request: yes
Labels: name: model card

# What does this PR do?
Added model cards for eight ELECTRA Tagalog models (four generators and four discriminators).

Comments:
@jcblaisecruz02 looks like your config.json files do not contain a `architectures` field nor a `model_type`, so your models might be incorrectly categorized – any way you could add those? Thank you!

> @jcblaisecruz02 looks like your config.json files do not contain a `architectures` field nor a `model_type`, so your models might be incorrectly categorized – any way you could add those? Thank you!

Ah gotcha! I'll add those. Thanks!

(easiest way should be to just call `.save_pretrained` again)
[examples] consistently use --gpus, instead of --n_gpu
Author: stas00
Pull request: yes

- some docs were wrongly suggesting to use `--n_gpu`, when the code is `--gpus`
- `examples/distillation/` had  `--n_gpu`, in the code - switched it and the doc to `--gpus`

Comments:
I had to update all the files since n_gpu seems like a common param.

You could create an issue, suggesting to use `n_gpu` everywhere instead, supporting it with some stats that would be in favor of this naming. As long as it's consistent across the project either way works, IMHO.