noamaanMulla-03/transformers-issues huggingface dataset:
BERT-viewer is broken for russian?
Author: Dronte
Pull request: no
Labels: name: wontfix

# 🐛 Bug
Hi I'm looking at BERT demos for russian, for example:
https://huggingface.co/DeepPavlov/rubert-base-cased?text=%D0%AF+%5BMASK%5D+%D1%81%D0%B0%D0%BB%D0%B0%D1%82+%D0%BD%D0%B0+%D0%BE%D0%B1%D0%B5%D0%B4.

English version for comparison:
https://huggingface.co/bert-base-cased?text=I+%5BMASK%5D+salad+for+lunch.

In the first link all token scores are 0.0 and they are absolute rubbish. Is everything fine with the viewer or the model?

Comments:
I don't see anything obviously wrong on our side, so you should ask the model authors (@deeppavlov and other members of @deepmipt) – maybe they uploaded the model without a trained final LM prediction layer, for instance.

Off topic, but e.g. by contrast this translation model works very well for russian: https://huggingface.co/Helsinki-NLP/opus-mt-ru-en?text=%D0%A3+%D0%BC%D0%B5%D0%BD%D1%8F+%D0%B1%D1%8B%D0%BB+%D1%81%D0%B0%D0%BB%D0%B0%D1%82+%D0%BD%D0%B0+%D0%BE%D0%B1%D0%B5%D0%B4

Hi!
We (DeepPavlov) might have a problem with Russian model that was uploaded to HugginFace and we need to check it.
Everything should be okay with MLM head if you use checkpoints from here:
http://docs.deeppavlov.ai/en/master/features/pretrained_vectors.html#downloads


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.
Updata tokenization_auto.py
Author: hjptriplebee
Pull request: yes

Updata tokenization_auto.py to handle Inherited config class.
If ConfigB is inherited from ConfigA, then isinstance(ConfigB(), ConfigA) is true. We hope to use TokenizerB, but TokenizerA will be used incorrectly.


Comments:
Hi, we have this [test](https://github.com/huggingface/transformers/blob/master/tests/test_configuration_auto.py#L45) to prevent exactly this. In what situation did you face an issue?

@LysandreJik There is no problem for pretrained models of huggingface transformers, because the config class of them are inherited from "PretrainedConfig". However, for users who want to add new models, their self-defined config class may be inherited from config class of some existing pretrained models. For example, I am trying to add a new model based on "BART" and my NewBartConig is inherited from "BartConig". My new tokenizer will not be used because a “NewBartConig” object is an instance of "BartConig" and bart tokenizer will be used incorrectly.

Yes, but we have similar issues with models, for example the `RobertaModel` inherits from `BertModel`. The test I mentioned above checks that (the example here is for configurations but we have the same test for models and tokenizers).

Currently the way to make sure your tokenizer is used and not the one on which it's depending is to put your tokenizer above the one it's inheriting from in the mapping. The for loop will then see this one first and use this one instead of the next one.

@LysandreJik Changing the order of items in TOKENIZER_MAPPING can solve the problem indeed. But get the rid of mapping order is more user-friendly, right? Close the pull request if you don't think the pr is necessary. Thanks for the review
Implement Layer-wise Relevance Propagation (LRP) for prediction explanation
Author: lapolonio
Pull request: no
Labels: name: wontfix

## 🚀 Feature

Example Code: 
https://github.com/lena-voita/the-story-of-heads/blob/master/lib/layers/attn.py#L154

## Motivation

The motivation is prediction explainability to be able to generate pictures like: 
![image](https://user-images.githubusercontent.com/1810412/72099957-b5fc3a00-32ef-11ea-8740-0d050809ea48.png)
or
![image](https://user-images.githubusercontent.com/1810412/72100012-d1674500-32ef-11ea-92ef-44917e08bf0b.png)

more motivation: http://www.heatmapping.org/slides/2019_ICCV.pdf


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.


I second this! :+1:
[s2s] pass max_length to config through command line
Author: sshleifer
Pull request: no

Problem:
In summarization, ideal beam search params vary between finetuning datasets. If you are finetuning pegasus-large on xsum, you want config.max_length=56, if you are finetuning pegasus-large on cnn-dailymail you want config.max_length=128. 

### Solutions
- the command line arg should be called `max_generate_length`

- This could also be addressed through adding `task_specific_params` for every dataset. Then you could pass `--task summarize_xsum` to finetune.py and things would work. Kinda lame though.

Comments:
Seq2SeqTrainer
Author: patil-suraj
Pull request: yes

This PR adds `Seq2SeqTrainer` to use `Trainer` for summarization, translation and other seq2seq tasks.

`Seq2SeqTrainer` includes:
- label smoothing loss
- sortish sampler
- predict from generate to allow calculating generative metrics

`finetune_trainer.py` includes:
- `Seq2SeqDataCollator` to correctly prepare `labels` and `decoder_input_ids`
- `Seq2SeqTrainingArguments` for extra training arguments.
- encoder and embedding freezing.
- `compute_metrics_fn` for translation(BLEU) and summarization (ROUGE)
- evaluation

Most of the data and model arguments are kept identical to `finetune.py` file 

Seq2Seq4Eva!🔥

Comments:
Command for running `finetune_trainer.py`

```bash
python finetune_trainer.py \
  --model_name_or_path sshleifer/bart-tiny-random \
  --data_dir xsum \
  --output_dir test \
  --overwrite_output_dir \
  --n_train 8 \
  --n_val 8 \
  --max_source_length 512 \
  --max_target_length 56 \
  --val_max_target_length 56 \
  --do_train \
  --do_eval \
  --num_train_epochs 2 \
  --per_device_train_batch_size 4 \
  --per_device_eval_batch_size 4 \
  --evaluate_during_training \
  --predict_from_generate \
  --logging_steps 2 \
  --save_steps 2 \
  --eval_steps 2 \
  --sortish_sampler \
```

Note: Eventually we need to refactor seq2seq/README.md to accommodate this

This looks great. Very excited to try this out with the `EncoderDecoder` model.

LGTM pending resolution of padding mystery.

Great work @patil-suraj !
how can i convert bert pytorch to tf2 ?
Author: ghost
Pull request: no

how can i convert bert pytorch to tf2 ?

Comments:
You can save it and reload it:

```py
pytorch_model.save_pretrained("here")
tf_model = TFBertModel.from_pretrained("here")
```

> You can save it and reload it:
> 
> ```python
> pytorch_model.save_pretrained("here")
> tf_model = TFBertModel.from_pretrained("here")
> ```

i can not convert pytorch bert model to tenforflow 2.3 ? can you help me ? @LysandreJik 

Please provide more information, can you respect the issue template? What exactly are you trying to do? Do you have a PyTorch model? How did you get it, is it one of the checkpoints on the hub, is it fine-tuned?

You want to convert it to our TensorFlow API? 

Please provide more details for us to help you better.
Bart encoder with add_final_layer_norm
Author: ruotianluo
Pull request: no

https://github.com/huggingface/transformers/blob/91cb95461e438dc57555c4f57f8ce95a56328036/src/transformers/modeling_bart.py#L305

I feel like it should be 
self.layer_norm = LayerNorm(config.d_model) if config.add_final_layer_norm else None

as in the BartDecoder.
https://github.com/huggingface/transformers/blob/91cb95461e438dc57555c4f57f8ce95a56328036/src/transformers/modeling_bart.py#L486


Comments:
Any idea?

You are correct. It will be fixed in the blenderbot PR if not sooner.
Bug in finetuning ALBERT on text-classification in GLUE
Author: ReyonRen
Pull request: no

Thanks for your beautiful work!

I use transformers-3.1.0, and when I run `run_glue.py` in `examples/text-classification/`, it return a bug info, 
```
Traceback (most recent call last):
  File "run_glue.py", line 246, in <module>
    main()
  File "run_glue.py", line 127, in main
    cache_dir=model_args.cache_dir,
  File "/mnt/dqa/yinheju/env/python3.7_torch1.1/lib/python3.7/site-packages/transformers/tokenization_auto.py", line 220, in from_pretrained
    return tokenizer_class_py.from_pretrained(pretrained_model_name_or_path, *inputs, **kwargs)
  File "/mnt/dqa/yinheju/env/python3.7_torch1.1/lib/python3.7/site-packages/transformers/tokenization_utils_base.py", line 1425, in from_pretrained
    return cls._from_pretrained(*inputs, **kwargs)
  File "/mnt/dqa/yinheju/env/python3.7_torch1.1/lib/python3.7/site-packages/transformers/tokenization_utils_base.py", line 1531, in _from_pretrained
    list(cls.vocab_files_names.values()),
OSError: Model name 'albert-base-v2' was not found in tokenizers model name list (albert-base-v1, albert-large-v1, albert-xlarge-v1, albert-xxlarge-v1, albert-base-v2, albert-large-v2, albert-xlarge-v2, albert-xxlarge-v2). We assumed 'albert-base-v2' was a path, a model identifier, or url to a directory containing vocabulary files named ['spiece.model'] but couldn't find such vocabulary files at this path or url.
```
Please help me to solve this problem, thank you!!!

Comments:
I have the same problem, if you have solved, cound you tell me how? thx!
[RAG] Fix retrieval offset in RAG's HfIndex and better integration tests
Author: lhoestq
Pull request: yes

Address @yjernite 's comment in https://github.com/huggingface/transformers/pull/7129#discussion_r488904472

Indeed the retriever was returning the indexes offset by one.

Cc @ola13 

Comments:
# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/7372?src=pr&el=h1) Report
> Merging [#7372](https://codecov.io/gh/huggingface/transformers/pull/7372?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/571c7a11c17bd00ba3e79f4d853cc51428a14e45?el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `88.88%`.

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

```diff
@@            Coverage Diff             @@
##           master    #7372      +/-   ##
==========================================
- Coverage   77.64%   77.63%   -0.01%     
==========================================
  Files         181      181              
  Lines       35722    35728       +6     
==========================================
+ Hits        27736    27738       +2     
- Misses       7986     7990       +4     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/7372?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/retrieval\_rag.py](https://codecov.io/gh/huggingface/transformers/pull/7372/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9yZXRyaWV2YWxfcmFnLnB5) | `91.01% <88.88%> (-0.27%)` | :arrow_down: |
| [src/transformers/generation\_tf\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/7372/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9nZW5lcmF0aW9uX3RmX3V0aWxzLnB5) | `85.46% <0.00%> (-0.76%)` | :arrow_down: |

------

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


@lhoestq - can you check how the `RUN_SLOW` tests would change in this case? 

> @lhoestq - can you check how the `RUN_SLOW` tests would change in this case?

They change indeed. I updated the expected values.

@yjernite - could you take a final look and approve if everything seems fine to you? 

Okey great - this should be the last big fix for RAG. I'll rebase this PR and merge it after
[wip] Send slack message if self-scheduled runner fails
Author: sshleifer
Pull request: yes


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

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

```diff
@@            Coverage Diff             @@
##           master    #4948      +/-   ##
==========================================
+ Coverage   77.14%   77.21%   +0.06%     
==========================================
  Files         128      128              
  Lines       21745    21745              
==========================================
+ Hits        16775    16790      +15     
+ Misses       4970     4955      -15     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/4948?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_tf\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/4948/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl91dGlscy5weQ==) | `87.26% <0.00%> (-0.16%)` | :arrow_down: |
| [src/transformers/modeling\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/4948/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ191dGlscy5weQ==) | `90.61% <0.00%> (+0.11%)` | :arrow_up: |
| [src/transformers/file\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/4948/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9maWxlX3V0aWxzLnB5) | `73.79% <0.00%> (+0.40%)` | :arrow_up: |
| [src/transformers/modeling\_xlnet.py](https://codecov.io/gh/huggingface/transformers/pull/4948/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ194bG5ldC5weQ==) | `78.65% <0.00%> (+2.29%)` | :arrow_up: |

------

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


@julien-c do you know an easy way to test whether this works without merging? 

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.
test_rag_sequence_generate_batch failing on CUDA
Author: sshleifer
Pull request: no
Labels: name: rag

https://github.com/huggingface/transformers/runs/1157849932?check_suite_focus=true
```
>       self.assertEqual(output_text_1, EXPECTED_OUTPUT_TEXT_1)
E       AssertionError: 'The song peaked at number 17 in the' != '"I Know Him So Well"'
```

Comments:
Yeah, If you run on CPU the test passes - I added a comment that the test fails on GPU: https://github.com/huggingface/transformers/blob/9e68d075a4100906509170498480823e7e61874a/tests/test_modeling_rag.py#L659

Beam search seems very sensible to small changes.

you mean sensitive, but OK. Maybe we should skip the test on CUDA so that slow CI isn't broken?

Actually, I fixed all `RagSequence` related bugs today and added better integration tests that should all pass on both CPU and GPU => so I think it's fine now.

See https://github.com/huggingface/transformers/blob/cf1c88e0921243e760d306e63a5938e1bac880f3/tests/test_modeling_rag.py#L664
Incorrect output fields names in docs
Author: visheratin
Pull request: no

## Environment info
     
- `transformers` version: 3.2.0
- Platform: Linux-5.4.0-7642-generic-x86_64-with-glibc2.29
- Python version: 3.8.2
- PyTorch version (GPU?): 1.6.0 (True)
- Tensorflow version (GPU?): not installed (NA)
- Using GPU in script?: yes
- Using distributed or parallel set-up in script?: no

### Who can help
@sgugger

## Information

The model I am using (Bert, XLNet ...): Bart

The problem arises when using the official example scripts.

```Python
from transformers import BartModel, BartTokenizer

tokenizer = BartTokenizer.from_pretrained('facebook/bart-base')
model = BartModel.from_pretrained('facebook/bart-base', return_dict=True, 
                                   output_hidden_states=True, output_attentions=True)

inputs = tokenizer("Hello, my dog is cute", return_tensors="pt")
outputs = model(**inputs)

print(outputs.hidden_states)
```

This script results in the error `'Seq2SeqModelOutput' object has no attribute 'hidden_states'`.

## To reproduce

Steps to reproduce the behavior:

1. Run the script above.

## Expected behavior

My expectation was to get a set of hidden states for the model. But in fact, the model returns two sets of hidden states - one for the decoder and another one for the encoder. It can be observed by looking at the keys of the `outputs`:

```Python
>>> print(outputs.keys())
odict_keys(['last_hidden_state', 'decoder_hidden_states', 'encoder_last_hidden_state', 'encoder_hidden_states'])
```

The same is valid for attentions if I specify `output_attentions=True`:

```Python
>>> print(outputs.keys())
odict_keys(['last_hidden_state', 'decoder_hidden_states', 'decoder_attentions', 'encoder_last_hidden_state', 'encoder_hidden_states', 'encoder_attentions'])
```

My conclusion is that the documentation gives an incorrect description of the output fields.

Comments:
Actually, the root of the problem might be related to the fact that the documentation states that the forward pass returns `BaseModelOutputWithPast` but in fact in returns `Seq2SeqModelOutput` ([source code](https://github.com/huggingface/transformers/blob/master/src/transformers/modeling_bart.py#L947-L955)).

Thanks for flagging! This should be fixed once the PR above is merged.

Solved by #7390
[trainer] Training from scratch
Author: sshleifer
Pull request: no

@patil-suraj is this possible in the new `Seq2SeqTrainer`? 


Possible solution sketch:
Where we call:

```
AutoSeq2SeqModelWithLMHead.from_pretrained(model_name)
```

Switch to 
```
if args.from_scratch: model = AutoSeq2SeqModelWithLMHead(config)
else: model = AutoSeq2SeqModelWithLMHead.from_pretrained(model_name)
```
What do you think?


Comments:
@sshleifer  Definitely possible, except we'll need to use `AutoModelForSeq2SeqLM` 😉
We can also pass a different `config` if we don't want to use `pretrained config` using `config_name` argument.

Happy to open a PR if it's needed :). Let me know

@sgugger is this possible with existing `Trainer? It seems like Seq2Seq is the wrong level for this feature to be implemented.

Models are initialised in example scripts rather than `Trainer`. Currently we need to save a from scratch model and then pass that. IMO it makes sense to add the `from_scratch` argument to `TrainingArguments` but each examples scripts will need to handle this itself


Oh that's a reasonable workaround. 

```
def save_randomly_initialized_version(config_name, save_dir, **config_kwargs):
    cfg = AutoConfig.from_pretrained(config_name, **config_kwargs)
    model = AutoModelForSeq2SeqLM.from_config(cfg)
    model.save_pretrained(save_dir)
    AutoTokenizer.from_pretrained(config_name).save_pretrained(save_dir)
```
I'll put this in the make_student PR.
Speedup check_copies script
Author: sgugger
Pull request: yes

Checking the copies by using black was slowing down the script quite a lot, so removing this check makes the script way faster. Removing `blackify` use could make the script less robust though, so leaving the function for now even if we don't use it anymore. If a situation arises where we see the script fail, I can code a (more complex) way of using black that would be fast.

With the two lines removed, the script takes 0.129s on my setup (instead of 18s).

cc @stas00 for information.

Comments:
Whoah! That's blazing fast! Thanks, @sgugger!

I think that's why `flake8` is slow - it runs black in some slow mode (`black` itself is very fast)

You can always add a flag that activates that disabled function, so it's there if needed.

This is no longer needed: `--line-length 119 --target-version py35` at https://github.com/huggingface/transformers/blob/90d1545f25b02a05b1581ae7a617db609fece0a0/utils/check_copies.py#L85
it now uses the config file - ensures we only have one place to do this setting.

Also, I haven't studied your code, but if it's applicable - skip checking files that haven't changes since last check - should give a huge speed increase, since typically only a few files are touched during a development of a single PR. If if is applicable and I can be of help let me know.

I wish black/flake8/isort did that too. It makes no sense to re-run the check on files that haven't changed, which is like 99% of files most of the time.

No need for a check-since-file-modified approach, use this instead:
```
git diff --name-only $(git merge-base --fork-point master)
```
as the source of what files to check.

It will give you all the files that were modified since the branch was made - yay!

But you only want specific sub-folders, so:

```
git diff --name-only $(git merge-base --fork-point master) | egrep '^(examples|templates|tests|src|utils)' | tr '\n' ' '
```

Now you can unleash whatever checks and it'd be all blazing fast.

I will post shortly a PR to make flake8 and other checkers rocket-fast! https://github.com/huggingface/transformers/pull/7403

I will make a function in Makefile which you can use to feed to the check scripts just the modified files. **edit**:  See https://github.com/huggingface/transformers/pull/7403 you now have a variable with all the modified files.
[Fix] Checkpoint saving and I/O for XLNetQASimple 
Author: jerrykuo7727
Pull request: yes
Labels: name: wontfix

**1.  [Fix] Create directories for model checkpoints**

Saving model checkpoints without making directories would lead to error. 
(i.e.  The output directory will not exist using the currenct script.)

The [older version](https://github.com/huggingface/transformers/blob/3f5ccb183e3cfa755dea2dd2afd9abbf1a0f93b8/examples/run_squad.py#L193) of the part of this script works fine.

**2.  [Fix] Fix wrong input/output structures for XLNet**

Running XLNet using the current script would lead to this error below.
> TypeError: forward() got an unexpected keyword argument 'cls_index'

The reason is that the [AutoModelForQuestionAnswering](https://github.com/huggingface/transformers/blob/ba2400189b2242620868096ae49babf93bd9ce00/examples/question-answering/run_squad.py#L735) would initialize XLNetForQuestionAnsweringSimple, while the I/O structures are written for XLNetForQuestionAnswering.

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.