noamaanMulla-03/transformers-issues huggingface dataset:
squad_convert_example_to_features does not work with CamembertTokenizer
Author: tbrendle
Pull request: no
Labels: name: Core: Tokenization

# 🐛 Bug

## Information

Model I am using : CamemBERT
Language I am using the model on : French

The problem arises when using:
* [*] my own modified scripts: (give details below)

The tasks I am working on is:
* [*] an official GLUE/SQUaD task: SQUaD

## To reproduce

Steps to reproduce the behavior:
1 - Copy paste this and run it
```python
from transformers import CamembertTokenizer, SquadExample, squad_convert_examples_to_features

tokenizer = CamembertTokenizer.from_pretrained('camembert-base')
example = SquadExample(
    'example_id',
    "Q",
    "C D E F G H",
    "D",
    2,
    "title"
)
features, _ = squad_convert_examples_to_features(
    examples=[
        example
    ],
    tokenizer=tokenizer,
    max_seq_length=30,
    doc_stride=128,
    max_query_length=128,
    is_training=True,
    return_dataset="pt",
    threads=1,
)

tokens = tokenizer.convert_ids_to_tokens(tokenizer.encode(example.question_text, example.context_text))
doc_token = example.doc_tokens

print({tokens[k]: doc_token[v] for k, v in features[0].token_to_orig_map.items()})
# Outputs 
# {'
': 'C', '▁C': 'D', '▁D': 'E', '▁E': 'F', '▁F': 'G', '▁G': 'H'}
# Should be
# {'▁C': 'C', '▁D': 'D', '▁E': 'E', '▁F': 'F', '▁G': 'G', '▁H': 'H'}
```

## Expected behavior

The resulting features mapping is shifted by one when using the CamembertTokenizer.

This seems to be caused by a weird check in the method `squad_convert_example_to_features`.  (`if "roberta" in str(type(tokenizer))`) is evaluated to False when using the CamembertTokenizer (which is adapted from RobertaTokenizer)

When I patch the line `if "roberta" in str(type(tokenizer))` by `if "roberta" in str(type(tokenizer)) or "camembert" in str(type(tokenizer))`, I get the expected behavior. 

I do not really know what would be the best way to handle this problem.

## Environment info

- `transformers` version: 2.4.1
- Platform: MacOS
- Python version: 3.7.6
- PyTorch version : 1.4
- Tensorflow version : None
- Using GPU in script?: No
- Using distributed or parallel set-up in script?: No


Comments:
Solved by #2746
Loading custom weights for BERT in pytorch
Author: LivC193
Pull request: no
Labels: name: Core: CLI

Is it possible to load custom pre-trained weights for BERT, other than the one you provide ? 

Comments:
Yes. Have a look at the documentation for [`from_pretrained`](https://huggingface.co/transformers/main_classes/model.html#transformers.PreTrainedModel.from_pretrained), particularly the `pretrained_model_name_or_path ` argument.

Yes sorry I looked after I asked and found everything I needed it
"Only evaluate when single GPU otherwise metrics may not average well"
Author: orenmelamud
Pull request: no
Labels: name: wontfix

Hi,

The script examples/run_lm_finetuning.py skips evaluation on the validation dataset when run in distributed mode on multiple GPUs. The code includes this comment regarding this:
"Only evaluate when single GPU otherwise metrics may not average well"

I'd appreciate it if someone could explain this issue in a few words and a maybe suggest a way around it? Can I simply run the evaluation on a single GPU (e.g. only for local_rank==0)?

Thanks!



Comments:
I have wondered about this comment as well. I have implemented multi-GPU evaluation and it works perfectly fine. By evaluation I mean that the the work of evaluating is distributed and all results are then gathered to the main GPU (e.g. 0) or CPU which then calculates loss and secondary metrics (f1/pearson). I haven't experienced any issues with it but perhaps there is a reason that I don't know about. 

Thanks, @BramVanroy. BTW, since you mentioned CPU, did you succeed in distributing LM fine-tuning on multiple CPUs? I tried that using torch.distributed and the 'gloo' backend and it seemed to be working fine, except that the total speed hardly improved.

Oh no, what I meant was doing some calculations such as correlations in the CPU. I've never done fine-tuning on CPU. I can imagine that it takes a long time. 

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 I was thinking of doing the same. Can you point me to your training script which uses multi-gpu eval the way you have described? I am hoping that if I see that first, I can avoid common mistakes. 
Thanks!

@dhruvdcoder Unfortunately, that code is in no state to be made public, in part because it is too complex and not incredibly well written. If I find the time, I plan to improve it and to add a PR here to update the example scripts to make use of multi-GPU evaluation.
Fix (non-slow) tests on GPU (torch)
Author: julien-c
Pull request: yes

Fixes ~29 failing tests

Also cf previous commit from december: https://github.com/huggingface/transformers/pull/2055/commits/61978c1dd3f340a545e74537c3dae41a4514e867

(not sure why T5 and the common methods were not failing back then)

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

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

```diff
@@           Coverage Diff           @@
##           master    #3024   +/-   ##
=======================================
  Coverage   77.26%   77.26%           
=======================================
  Files          98       98           
  Lines       16047    16047           
=======================================
  Hits        12399    12399           
  Misses       3648     3648
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/3024?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_bart.py](https://codecov.io/gh/huggingface/transformers/pull/3024/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19iYXJ0LnB5) | `84.58% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/3024?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/transformers/pull/3024?src=pr&el=footer). Last update [bb7c468...9c14bc7](https://codecov.io/gh/huggingface/transformers/pull/3024?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
fix several typos in Distil* readme
Author: awalker88
Pull request: yes

Hi, feel free to reject if you think these changes aren't necessary. Just saw a couple typos while reading the documentation and wanted to help 😄. The first change looks a bit hard to tell what changed, but it was 'superseeds' to 'supersedes'.

Comments:
# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/3034?src=pr&el=h1) Report
> Merging [#3034](https://codecov.io/gh/huggingface/transformers/pull/3034?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/9df74b8bc42eedc496f7148b9370728054ca3b6a?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master    #3034      +/-   ##
==========================================
+ Coverage   77.27%   77.29%   +0.01%     
==========================================
  Files          98       98              
  Lines       16037    16037              
==========================================
+ Hits        12393    12395       +2     
+ Misses       3644     3642       -2
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/3034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/3034/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ191dGlscy5weQ==) | `92.54% <0%> (+0.32%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/3034?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/transformers/pull/3034?src=pr&el=footer). Last update [9df74b8...1482c03](https://codecov.io/gh/huggingface/transformers/pull/3034?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
Recently added pipelines tests should be marked as slow
Author: aaugustin
Pull request: no
Labels: name: wontfix

## 🐛 Bug

Starting today, when running tests, some very large files are downloaded even though I don't enable RUN_SLOW=true.

Some tests in pipelines_test.py should be marked with `@slow` so they don't run unless RUN_SLOW=True.

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.
problem in the doc, in the  "Quick Start" GPT2 example
Author: t-laurent
Pull request: no
Labels: name: wontfix

I am going through the GPT2 example in the doc. Is there a mistake in the "Using the past" code. The main loop to generate text is:

```python
for i in range(100):
    print(i)
    output, past = model(context, past=past)
    token = torch.argmax(output[0, :])
    generated += [token.tolist()]
    context = token.unsqueeze(0)
```


At the first iteration the tensor output as shape [1, 3, 50257], and at all the following iterations it has size [1,50257]. Should the code be:

```python
for i in range(100):
    print(i)
    output, past = model(context, past=past)
    if i==0:
        token = torch.argmax(output[0,-1,:])
    else:
        token = torch.argmax(output[0, :])
    generated += [token.tolist()]
    context = token.unsqueeze(0)
```







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.
[WIP] Updates to simplify PLT example and use new features
Author: srush
Pull request: yes

Some changes to PLT to support our workflow. Prelim support for TPUs. 

Still testing. 

Comments:
I can't seem to find who is pushing the commits, but can whoever is doing that please add commit messages? This is really messy and impossible to go over as currently written.

Sorry, @BramVanroy . I didn't realize people were reviewing this branch. This is a WIP We are still trying to work this out with new pytorch-lightning / TPU changes. (Closing for now and will switch to a new review branch when it is working). 

@srush No worries! Wasn't really reviewing, I was just curious which changes were being made - but then I saw those commit messages and I didn't know what to expect. 😄
Possible improvement of padding logic in generate
Author: patrickvonplaten
Pull request: yes

I wanted to try out a approach to the padding logic (previously discussed with @thomwolf and @mfuntowicz) which is **different** to what is currently done (see #2885 description). 

Instead of setting the `pad_token_id` to `eos_token_id` if `pad_token_id` is not defined and `eos_token_id` is defined, the following logic could be applied:

If there is no `pad_token_id`, the user has to add the `pad_token_id` to the tokenizer and resize the model token embedding matrix to add an additional token vector to the weight matrix.

During my PR today, I encountered two problems:

1. Adding a token embedding vector causes some pretrained model (`gpt2` and `openai-gpt`) often generate the newly added token embedding vector (since input & output token embeddings are tied). A remedy for this is to simply always set the produced logits of the pad_token_id to -Inf. This works as it doesn't generate the `pad_token`, but it changes the internal behavior of `gpt2` and `openai-gpt` however. 

2. TransfoXL uses an adaptive token embedding matrix, which means that the function `resize_embedding_matrix` produces an error when used with `TransfoXLHeadModel`. This means the `token_embedding_matrix` of TransfoXL cannot be changed! Therefore, this approach doesn't work at all for TransfoXL (which is a pity as TransfoXL generates very good language). 

In the current PR, some tests fail (TransfoXL and the slow tests of gpt2 and openai-gpt). I wanted to hear your opinion on this logic vs. the logic that is implemented now (see #2885) @thomwolf, @LysandreJik and @mfuntowicz 


Comments:
Fix batch_encode_plus
Author: astariul
Pull request: yes

Fix #3037 

Comments:
# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/3041?src=pr&el=h1) Report
> Merging [#3041](https://codecov.io/gh/huggingface/transformers/pull/3041?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/b370cc7e99c5b8c7436154d4694c33b461ea0f08?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master    #3041      +/-   ##
==========================================
+ Coverage   77.28%   77.29%   +<.01%     
==========================================
  Files          98       98              
  Lines       16038    16037       -1     
==========================================
  Hits        12395    12395              
+ Misses       3643     3642       -1
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/3041?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/tokenization\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/3041/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fdXRpbHMucHk=) | `91.7% <100%> (+0.13%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/3041?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/transformers/pull/3041?src=pr&el=footer). Last update [b370cc7...59d9a21](https://codecov.io/gh/huggingface/transformers/pull/3041?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
Wrong logic in `batch_encode_plus()`
Author: astariul
Pull request: no

# 🐛 Bug

I'm trying to use `batch_encode_plus()` with already tokenized ID as input.

So I have a `list of list of int` as input. For example :

```python
x = [[0, 83, 313, 11, 10551, 2278, 16, 2183, 1958, 804, 7, 916, 11, 13933, 982, 4, 286, 68, 5046, 6, 37, 40, 3627, 231, 2697, 9, 1958, 11, 41, 36027, 27894, 1001, 21466, 424, 2233, 4, 2], [0, 20, 291, 212, 13989, 191, 3772, 42, 983, 4, 815, 34, 1714, 8617, 187, 63, 17692, 11, 8008, 4, 993, 864, 549, 1492, 2624, 5391, 9686, 8, 12291, 240, 7, 464, 4, 2]]
```

When I call `batch_encode_plus()` with this input, I receive `AssertionError` :

```
tokenizer.batch_encode_plus(x)
# >>> line 1130, in batch_encode_plus
#                    assert len(ids_or_pair_ids) == 2
```

---

After reviewing the code, it seems there is an error in the logic. Here :

https://github.com/huggingface/transformers/blob/b370cc7e99c5b8c7436154d4694c33b461ea0f08/src/transformers/tokenization_utils.py#L1128-L1137

since I gave only tokenized ID and not a tuple of tokenized ID (not a pair), the logic should bring me to the `else` clause. But instead, I enter the `if` clause, leading to the assertion error.

**It's currently not possible to use `batch_encode_plus()` with already tokenized text / IDs** if the input is not a pair.

---

I think this line :

https://github.com/huggingface/transformers/blob/b370cc7e99c5b8c7436154d4694c33b461ea0f08/src/transformers/tokenization_utils.py#L1129

Should be changed to this :

```python
if isinstance(ids_or_pair_ids, (list, tuple)) and len(ids_or_pair_ids) == 2:
```


Comments:
Changing the loss function in BERT
Author: paul-you
Pull request: no
Labels: name: Usage

# ❓ Questions & Help

Hello, 

I'm trying to replace the _CrossEntropyLoss_ with the _KLDivLoss_ for a sentence classification task using some golden logits and the logits from the BERT model.

```
golden_logits = …

outputs = model(**inputs)

# the logits from BERT
logits = outputs[1]

loss_fct = KLDivLoss(reduction='sum')
loss = loss_fct(F.log_softmax(logits, dim=-1), F.softmax(golden_logits, dim=-1))
```

Am I doing this correctly ? 

Comments:
This is a very general PyTorch question. Please post it on Stack Overflow. https://stackoverflow.com/

About the BERT part: `outputs` is a tuple. `[0]`  is the last hidden state (sequence output), `[1]` is the pooled output.
why this implementation didn't apply residual and layer norm?
Author: reniew
Pull request: no

## ❓ Questions & Help

In ALBERT implementation code `modeling_albert.py`, i can't find applying skip-connection and layer normalizing after multi-head attention layer. I didn't read about this tequnique. 
Is there a special reason about it?

One more, i saw the argument `inner_group_num` in `AlbertLayerGroup` class, is it introduced in original ABERT paper?

Thank you for good implementing.


Comments:
The layer normalization happening after the attention is visible [here](https://github.com/huggingface/transformers/blob/master/src/transformers/modeling_albert.py#L255).

The `inner_group_num` is used to better understand how many layers are in a specific group.  It is set to 1 in all the configurations that google-research has output as they all have a single repeating layer, but it would be necessary to increase the number of inner groups if you pre-trained an ALBERT model that used more than one repeating layer.

You can see the `inner_group_num` in the official configuration files, for example the [xxlarge-v3](https://tfhub.dev/google/albert_xxlarge/3).
Adding usage examples for common tasks
Author: LysandreJik
Pull request: yes

Adding a documentation page detailing usage for common tasks (inference, not training

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

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

```diff
@@           Coverage Diff           @@
##           master    #2850   +/-   ##
=======================================
  Coverage   75.06%   75.06%           
=======================================
  Files          94       94           
  Lines       15287    15287           
=======================================
  Hits        11475    11475           
  Misses       3812     3812
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/2850?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/transformers/pull/2850?src=pr&el=footer). Last update [7f98edd...51830ef](https://codecov.io/gh/huggingface/transformers/pull/2850?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


I've added a way to switch between `PyTorch` and `TensorFlow` implementations. I didn't want to have a wall of code that had the two frameworks, so now there's a toggle to show which framework you would like to see.

It works as follows: a javascript method parses the documentation page shown and looks for `.highlight` classes, which are the code blocks. In there, it looks for `## PYTORCH CODE`, which represents the beginning of a `PyTorch` snippet and `## TENSORFLOW CODE` which represents the beginning of a `TensorFlow` snippet.

Would love an opinion on the Javascript code as well. Would love to convert this to TS down the road.

Here's a gif of the result
![peek3](https://user-images.githubusercontent.com/30755778/75115468-db899c80-562c-11ea-81aa-28cabf6af538.gif)



@LysandreJik reviewing the JS now and had a question. Depending on your thoughts, would it make sense from a UX standpoint for all of the buttons to toggle together? So, if a user selects "Tensorflow" all of the code blocks would switch to "Tensorflow". 

I guess this would be cool and makes sense from a UX standpoint. Do you think it's necessary or can it wait for the second version?

It can 100% wait for a second version.
Does the calling order need to be changed?
Author: bcmi220
Pull request: no
Labels: name: wontfix

## ❓ Questions & Help
pytorch: 1.3.0

torch/optim/lr_scheduler.py:100: UserWarning: Detected call of `lr_scheduler.step()` before `optimizer.step()`. In PyTorch 1.1.0 and later, you should call them in the opposite order: `optimizer.step()` before `lr_scheduler.step()`.  Failure to do this will result in PyTorch skipping the first value of the learning rate schedule.See more details at https://pytorch.org/docs/stable/optim.html#how-to-adjust-learning-rate
  "https://pytorch.org/docs/stable/optim.html#how-to-adjust-learning-rate", UserWarning)


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.
Is ALBERT the right implement from paper ?  
Author: adamluo1995
Pull request: no

I read ALBERT paper and code form Google: github.com/google-research/ALBERT. One of the main contribute of ALBert is cross-layer parameter sharing and i can see it on Google's code. But i can't see sharing on this code. Every Layers(or Blocks) are new object and their parameter will be different after fine-tuning.

Is the implement is wrong or i have misunderstanding about parameter sharing?

Comments:
I read the code carefully, and it do share.