noamaanMulla-03/transformers-issues huggingface dataset:
fix #878
Author: xijiz
Pull request: yes


Comments:
# [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/879?src=pr&el=h1) Report
> Merging [#879](https://codecov.io/gh/huggingface/pytorch-transformers/pull/879?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/pytorch-transformers/commit/067923d3267325f525f4e46f357360c191ba562e?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `0%`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/pytorch-transformers/pull/879/graphs/tree.svg?width=650&token=9qOlN6Hb1c&height=150&src=pr)](https://codecov.io/gh/huggingface/pytorch-transformers/pull/879?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #879      +/-   ##
==========================================
- Coverage   79.03%   79.02%   -0.02%     
==========================================
  Files          34       34              
  Lines        6234     6235       +1     
==========================================
  Hits         4927     4927              
- Misses       1307     1308       +1
```


| [Impacted Files](https://codecov.io/gh/huggingface/pytorch-transformers/pull/879?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pytorch\_transformers/tokenization\_utils.py](https://codecov.io/gh/huggingface/pytorch-transformers/pull/879/diff?src=pr&el=tree#diff-cHl0b3JjaF90cmFuc2Zvcm1lcnMvdG9rZW5pemF0aW9uX3V0aWxzLnB5) | `86.17% <0%> (-0.36%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/879?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/879?src=pr&el=footer). Last update [067923d...31bc1dd](https://codecov.io/gh/huggingface/pytorch-transformers/pull/879?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Closing this for now. Feel free to re-open if you want to continue with this PR.
Fix #1015 (tokenizer defaults to use_lower_case=True when loading from trained models)
Author: qipeng
Pull request: yes

This PR fixes the issue where the tokenizer always defaults to `use_lower_case=True` when loading from trained models. It returns the control to the command-line arguments.

Comments:
# [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1055?src=pr&el=h1) Report
> Merging [#1055](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1055?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/pytorch-transformers/commit/c589862b783b94a8408b40c6dc9bf4a14b2ee391?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1055/graphs/tree.svg?width=650&token=9qOlN6Hb1c&height=150&src=pr)](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1055?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           master   #1055   +/-   ##
======================================
  Coverage    79.6%   79.6%           
======================================
  Files          42      42           
  Lines        6863    6863           
======================================
  Hits         5463    5463           
  Misses       1400    1400
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1055?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1055?src=pr&el=footer). Last update [c589862...3bffd2e](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1055?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


That's great, thanks @qipeng.

Do you think you could do the same fix on the  `run_glue` example?

Added tokenizer fix in `run_glue.py` and fixed `do_train` logic in `run_squad.py`

Great, thanks a lot @qipeng!
Logic issue with evaluating cased models in `run_squad.py`
Author: qipeng
Pull request: no

## 🐛 Bug

<!-- Important information -->

Model I am using (Bert, XLNet....): Bert

Language I am using the model on (English, Chinese....): English

The problem arise when using:
* [x] the official example scripts: (give details) `run_squad.py` with cased models
* [ ] my own modified scripts: (give details)

The tasks I am working on is:
* [x] an official GLUE/SQUaD task: (give the name) squad
* [ ] my own task or dataset: (give details)

## To Reproduce

Steps to reproduce the behavior:

1. Finetune a cased model with `--do_train` and `--do_eval` (the latter is optional)
2. Use `--do_eval` to make predictions.

## Expected behavior

<!-- A clear and concise description of what you expected to happen. -->

At evaluation time, the tokenizer should also be cased, but because it's loading from a path and not using a model name, the `from_pretrained` method in `BertTokenizer` fails to identify casing information, and the `BasicTokenizer` defaults to uncased (`do_lower_case=True`).

## Environment

(most of this is probably not relevant anyway)

* OS: Ubuntu 16.04
* Python version: 3.6.8
* PyTorch version: 1.2.0
* PyTorch Transformers version (or branch): master
* Using GPU ? Yes
* Distributed of parallel setup ? DataParallel
* Any other relevant information:

## Additional context


One solution is to add `do_lower_case=args.do_lower_case` in the kwargs here:
https://github.com/huggingface/pytorch-transformers/blob/master/examples/run_squad.py#L501

Comments:
Thanks for the bug report, will look into it.

Looks good to me, do you want to push a PR to fix this as you proposed @qipeng?

Done. See #1055!

Found another one: https://github.com/huggingface/pytorch-transformers/blob/master/examples/run_squad.py#L484
It seems like it should be 
```python
if args.do_train and (args.local_rank == -1 or torch.distributed.get_rank() == 0):
```
instead of
```python
if args.do_train and args.local_rank == -1 or torch.distributed.get_rank() == 0:
```
?
I.e., this block shouldn't go through unless `args.do_train` is set explicitly IMO.

Yes, good catch @qipeng and thanks for the PR, do you want to add this fix to your PR as well?

Updated my PR!
Better use of spacy tokenizer in open ai and xlm tokenizers
Author: GuillemGSubies
Pull request: yes

When you do: `spacy.load('en', disable=['parser', 'tagger', 'ner', 'textcat'])` There is a high risk of throwing an exception if the user did not install the model before.

Te easiest way to use the spaCy tokenizer is the one I propose here. This way there is no need for the user to download any spaCy model.

More info here: https://spacy.io/api/tokenizer#init 

Comments:
# [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059?src=pr&el=h1) Report
> Merging [#1059](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/pytorch-transformers/commit/933841d903a032d93b5100220dc72db9d1283eca?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `0%`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059/graphs/tree.svg?width=650&token=9qOlN6Hb1c&height=150&src=pr)](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1059      +/-   ##
==========================================
- Coverage    79.6%   79.57%   -0.03%     
==========================================
  Files          42       42              
  Lines        6863     6865       +2     
==========================================
  Hits         5463     5463              
- Misses       1400     1402       +2
```


| [Impacted Files](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pytorch\_transformers/tokenization\_openai.py](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059/diff?src=pr&el=tree#diff-cHl0b3JjaF90cmFuc2Zvcm1lcnMvdG9rZW5pemF0aW9uX29wZW5haS5weQ==) | `81.51% <0%> (-0.7%)` | :arrow_down: |
| [pytorch\_transformers/tokenization\_xlm.py](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059/diff?src=pr&el=tree#diff-cHl0b3JjaF90cmFuc2Zvcm1lcnMvdG9rZW5pemF0aW9uX3hsbS5weQ==) | `83.06% <0%> (-0.68%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059?src=pr&el=footer). Last update [933841d...388e325](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1059?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Indeed!
Getting embedding from XLM in differnet languages
Author: OfirArviv
Pull request: no

## ❓ Questions & Help

Hi,

I'm trying to get a cross-lingual embedding from the XLM model, but can't figure out how.
In the project original github, you need to give the tokenizer the language of each of the tokens, but it doesn't seem the case here.
Will appreciated any help on the matter :)

Comments:
Hi, are you referring to the sentence embeddings that are generated in the [XLM notebook](https://github.com/facebookresearch/XLM/blob/master/generate-embeddings.ipynb)?

Yes. Although I'm using it to get the word embeddings (see [here](https://github.com/facebookresearch/XLM/issues/17)).
Maybe I'm missing something, but as far as I understand, the model uses a language embedding  that is added to the token embedding, so it seem it will need that information. Am I missing something?

Well, neither the official XLM notebook @LysandreJik linked to nor the XLM repo issue @OfirArviv linked to are mentioning the need to give language ids so I'm not sure exactly why they would be needed.

Maybe this is a question for the original authors of XLM?

Hi @thomwolf!

I believe they already answered this question in [this](https://github.com/facebookresearch/XLM/issues/103#issuecomment-501682382) [issue](https://github.com/facebookresearch/XLM/issues/103#issuecomment-501682649): 

So it will be useful if we can provide models with lang ids, preferably during training as well.

  


Ok I see.

So you need to input a `torch.LongTensor` with the `language id` for each token in your input sequence in the model (see inputs [here](https://huggingface.co/pytorch-transformers/model_doc/xlm.html#pytorch_transformers.XLMModel)).

Right now the conversion mapping from language to ids (and vice-versa) can be found in the configuration of the model (see for ex [here](https://s3.amazonaws.com/models.huggingface.co/bert/xlm-mlm-xnli15-1024-config.json)).

Here is an example:
```python
from pytorch_transformers import XLMModel
model = XLMModel.from_pretrained('xlm-mlm-xnli15-1024')
lang2id_dict = model.config.lang2id
id2lang_dict = model.config.id2lang
```

If you only want the conversion dictionary and not the model, just load only the configuration:
```python
from pytorch_transformers import XLMConfig
config = XLMConfig.from_pretrained('xlm-mlm-xnli15-1024')
lang2id_dict =config.lang2id
id2lang_dict =config.id2lang
```

I'll add more details on that in the docstring.
BertTokenizer.save_vocabulary() doesn't work as docstring described
Author: boy2000-007man
Pull request: no

## 🐛 Bug
https://github.com/huggingface/pytorch-transformers/blob/1b35d05d4b3c121a9740544aa6f884f1039780b1/pytorch_transformers/tokenization_bert.py#L169-L174

## Expected behavior

It's obvious that when `vocab_path` is not a directory, the `vocab_file` is not defined.
I believe replacing all `vocab_path` with `vocab_file` solves this issue, vice versa.

Comments:
Example in OpenAIGPTDoubleHeadsModel can't run
Author: HaokunLiu
Pull request: no

I tried to run the example from OpenAIGPTDoubleHeadsModel. But it went wrong.
Although the tokenizer added new index for special tokens, the embedding in OpenAIGPTDoubleHeadModel didn't add new embeddings for them, which leads to index out of range.

``` 
tokenizer = OpenAIGPTTokenizer.from_pretrained('openai-gpt') 
model = OpenAIGPTDoubleHeadsModel.from_pretrained('openai-gpt')
tokenizer.add_special_tokens({'cls_token': '[CLS]'})  # Add a [CLS] to the vocabulary (we should train it also!)
choices = ["Hello, my dog is cute [CLS]", "Hello, my cat is cute [CLS]"]
input_ids = torch.tensor([tokenizer.encode(s) for s in choices]).unsqueeze(0)  # Batch size 1, 2 choices
mc_token_ids = torch.tensor([input_ids.size(-1), input_ids.size(-1)]).unsqueeze(0)  # Batch size 1
outputs = model(input_ids, mc_token_ids)
lm_prediction_scores, mc_prediction_scores = outputs[:2]
```

Comments:
Yes, you need to resize the embeddings as well.
There is [an example](https://huggingface.co/pytorch-transformers/main_classes/tokenizer.html#pytorch_transformers.PreTrainedTokenizer.add_special_tokens) in the doc of the `add_special_tokens`method, that I copy here:
```
# Let's see how to add a new classification token to GPT-2
tokenizer = GPT2Tokenizer.from_pretrained('gpt2')
model = GPT2Model.from_pretrained('gpt2')

special_tokens_dict = {'cls_token': '<CLS>'}

num_added_toks = tokenizer.add_special_tokens(special_tokens_dict)
print('We have added', num_added_toks, 'tokens')
model.resize_token_embeddings(len(tokenizer))  # Notice: resize_token_embeddings expect to receive the full size of the new vocabulary, i.e. the length of the tokenizer.

assert tokenizer.cls_token == '<CLS>'
```

Ah, I see. Now it works. Thanks a lot.
"mask_padding_with_zero" for xlnet
Author: tbornt
Pull request: no

## ❓ Questions & Help

<!-- A clear and concise description of the question. -->
From the source code in [xlnet repo](https://github.com/zihangdai/xlnet/blob/master/classifier_utils.py) line113-115
I see the comment
`
 The mask has 0 for real tokens and 1 for padding tokens. Only real
 tokens are attended to.

input_mask = [0] * len(input_ids)
`

But in this repo, I found the code for generate input_mask in examples/utils_glue.py
`
input_mask = [1 if mask_padding_with_zero else 0] * len(input_ids)
`
and `mask_padding_with_zero` for xlnet and bert is all set True.

I'm confused if this is a bug.

Comments:
It is not. We've added an option to input a negative mask in XLNet so it can use the same input pattern as the other models.

If you take a look at the inputs of XLNetModel [here](https://huggingface.co/pytorch-transformers/model_doc/xlnet.html#pytorch_transformers.XLNetModel), you will see both possible masks: `attention_mask` (the original XLNet mask), `input_mask` the negative we use in the SQuAD example.

Oh, I see. Great work to maintain consistency with other models.
GPT2 model does not have attention mask
Author: Saner3
Pull request: no

Hello, in the doc string of GPT2 model, it says there is an optional input called [attention_mask](https://github.com/huggingface/pytorch-transformers/blob/f289e6cfe46885f260e4f2b3c8a164aa1a567e4c/pytorch_transformers/modeling_gpt2.py#L405) to avoid computing attention on paddings. But actually I cannot find the implementation and there is no such arguments either. 

Comments:
Indeed, I will remove this doctring, there is no attention_mask on GPT-2.

> Indeed, I will remove this doctring, there is no attention_mask on GPT-2.

But what to do if I do want to avoid computing attention on the paddings in the input sequences.

@Saner3 @thomwolf I have same question? don't we need that for paddings?

GPT-2 is a model with absolute position embeddings (like Bert) so you should always pad on the right to get best performances for this model (will add this information to the doc_string).

As it's a causal model (only attend to the left context), also means that the model will not attend to the padding tokens (which are on the right) for any real token anyway.

So in conclusion, no need to take special care of avoiding attention on padding.

Just don't use the output of the padded tokens for anything as they don't contain any reliable information (which is obvious I hope).

@thomwolf thanks much, and great job!
BUG: run_openai_gpt.py bug of GPTTokenizer and GPTDoubleHeadsModel
Author: liyucheng09
Pull request: no

## 🐛 Bug

<!-- Important information -->

Model I am using (Bert, XLNet....): GPT
Language I am using the model on (English, Chinese....): ENGLISH

The problem arise when using:
* [ 1 ] the official example scripts: run_openai_gpt.py

The tasks I am working on is:
* [ 1 ] an official GLUE/SQUaD task:  ROCSroties

**Run run_openai_gpt.py file error, Traceback are as fellow:**
```
Traceback (most recent call last):
  File "/opt/lyon.li/gpt-2/examples/single_model_scripts/run_openai_gpt.py", line 288, in <module>
    main()
  File "/opt/lyon.li/gpt-2/examples/single_model_scripts/run_openai_gpt.py", line 158, in main
    model = OpenAIGPTDoubleHeadsModel.from_pretrained(args.model_name, num_special_tokens=len(special_tokens))
  File "/opt/lyon.li/gpt-2/pytorch_transformers/modeling_utils.py", line 474, in from_pretrained
    model = cls(config, *model_args, **model_kwargs)
TypeError: __init__() got an unexpected keyword argument 'num_special_tokens'
```

It seems like GPTTokenizer and GPTModel fail to add special tokens. Run fellowing demo give me result like this.
```
special_tokens = ['_start_', '_delimiter_', '_classify_']
tokenizer = OpenAIGPTTokenizer.from_pretrained(args.model_name, special_tokens=special_tokens)
special_tokens_ids = list(tokenizer.convert_tokens_to_ids(token) for token in special_tokens)
```
return
```
special_tokens_ids=[0, 0, 0]
```
That means every special tokens was mapped to unk_token when tokenize.
When init GPTModel it directly report error because `num_special_tokens`.

Anyone have some ideas about why it dose not works, thanks.

Comments:
Yes, the `run_openai_gpt.py` example still needs to be updated to the new pytorch-transformers release. We haven't found time to do it yet.

I have pull request at #1067, this change fix the bug I mentioned above.
BUG: run_openai_gpt.py load ROCStories data error
Author: liyucheng09
Pull request: no

## 🐛 Bug

Model I am using (Bert, XLNet....): GPT

Language I am using the model on (English, Chinese....): English

The problem arise when using:
* [ 1 ] the official example scripts: run_openai_gpt.py

The tasks I am working on is:
* [ 1 ] an official GLUE/SQUaD task: ROCStories

**The method of preprocess of ROCStories use ids 0 pad input, while 0 is actually the id of unk_tokens.**

The code of method is as fellowing:
```
def pre_process_datasets(encoded_datasets, input_len, cap_length, start_token, delimiter_token, clf_token):
    tensor_datasets = []
    for dataset in encoded_datasets:
        n_batch = len(dataset)
        input_ids = np.zeros((n_batch, 2, input_len), dtype=np.int64)
        for i, (story, cont1, cont2, mc_label), in enumerate(dataset):
            with_cont1 = [start_token] + story[:cap_length] + [delimiter_token] + cont1[:cap_length] + [clf_token]
            with_cont2 = [start_token] + story[:cap_length] + [delimiter_token] + cont2[:cap_length] + [clf_token]
            input_ids[i, 0, :len(with_cont1)] = with_cont1
            input_ids[i, 1, :len(with_cont2)] = with_cont2
```
input_ids initialize with 0, which is the id of unk_token, rather than id of pad_token.

Comments:
Hi @nine09, thanks for the report. Any way you could fix it cleanly and open a pull request?

> Hi @nine09, thanks for the report. Any way you could fix it cleanly and open a pull request?

Bien sur! But I want to have some clues about whether GPTTokenizer already have pad_token, otherwise add a new pad_token need resize embedding of GPTModel.

I have fix the bug at #1067, that change add a pad_token to GPTTokenizer so that solved this problem.
Add 117M and 345M as aliases for pretrained models
Author: yet-another-account
Pull request: yes

This keeps better with the convention in the tensorflow repository. 

Comments:
# [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/952?src=pr&el=h1) Report
> Merging [#952](https://codecov.io/gh/huggingface/pytorch-transformers/pull/952?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/pytorch-transformers/commit/44dd941efb602433b7edc29612cbdd0a03bf14dc?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/pytorch-transformers/pull/952/graphs/tree.svg?width=650&token=9qOlN6Hb1c&height=150&src=pr)](https://codecov.io/gh/huggingface/pytorch-transformers/pull/952?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #952   +/-   ##
=======================================
  Coverage   79.04%   79.04%           
=======================================
  Files          34       34           
  Lines        6242     6242           
=======================================
  Hits         4934     4934           
  Misses       1308     1308
```


| [Impacted Files](https://codecov.io/gh/huggingface/pytorch-transformers/pull/952?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pytorch\_transformers/modeling\_gpt2.py](https://codecov.io/gh/huggingface/pytorch-transformers/pull/952/diff?src=pr&el=tree#diff-cHl0b3JjaF90cmFuc2Zvcm1lcnMvbW9kZWxpbmdfZ3B0Mi5weQ==) | `75.84% <ø> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/952?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/952?src=pr&el=footer). Last update [44dd941...d40e827](https://codecov.io/gh/huggingface/pytorch-transformers/pull/952?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Thanks, I think we'll stick with `gpt2`, `gpt2-medium` and `gpt2-large` for now.
(also because these number of parameters are actually wrong, the models are respectively 124M and 355M parameters as indicated in the [updated readme of gpt-2](https://github.com/openai/gpt-2#gpt-2))