Clean special token init in modeling_....py
Author: patrickvonplaten
Pull request: yes

#### INTRO:
This PR is a follow-up from PR #3011. 

After discussion with @thomwolf today, we decided that the variable `eos_token_ids` in all models causes more confusion and ugly code than it helps. 

#### BACKGROUND: 
All models now have `pad_token_id`, `bos_token_id` and `eos_token_id` as default values. The reasons are discussed and explained in #3011.

Originally, we had the `list` variable `eos_token_ids`. The idea behind was that a model could have multiple `eos_token_ids` if the user wants to finish at certain tokens besides the standard EOS token. But this caused a lot of unclean code AND is not consistent with `tokenizers` which all has a `tokenizer.eos_token_id` int variable. So, we return to `eos_token_id` for moders as well and might in the future have a variable `forbidden_tokens` or `special_stop_tokens`. 

#### THIS PR DOES:
- Replace all list `eos_token_ids` with `eos_token_id`
- Add default `eos_token_id, pad_token_id, bos_token_id` to all models

#### TESTS: 
I tested that the `pretrained Config` has now the same special tokens as the `pretrained Tokenizer` for all model identifier names (e.g. `gpt2-large`) with the following code:

```
for model_id_name in ALL_PRETRAINED_MODEL_ARCHIVE_MAP.keys():
    tok = AutoTokenizer.from_pretrained(model_id_name)
    conf = AutoConfig.from_pretrained(model_id_name)

    pad_equal = tok.pad_token_id == conf.pad_token_id
    eos_equal = tok.eos_token_id == conf.eos_token_id
    bos_equal = tok.bos_token_id == conf.bos_token_id

    if not pad_equal:
        print("PAD not equal for {}!".format(model_id_name))
        print("TOK: {} | CONF: {}".format(tok.pad_token_id, conf.pad_token_id))

    if not eos_equal:
        print("EOS not equal for {}!".format(model_id_name))
        print("TOK: {} | CONF: {}".format(tok.eos_token_id, conf.eos_token_id))

    if not bos_equal:
        print("BOS not equal for {}!".format(model_id_name))
        print("TOK: {} | CONF: {}".format(tok.bos_token_id, conf.bos_token_id))
```

which gives the following result: 

```
PAD not equal for bert-base-dutch-cased!
TOK: 3 | CONF: 0
BOS not equal for distilbert-base-cased!
TOK: None | CONF: 0
BOS not equal for distilbert-base-cased-distilled-squad!
TOK: None | CONF: 0
```

This means that: 
- `bert-base-dutch-cased` has a different `pad_token_id` in its tokenizer config than the `pad_token_id` in default Bert tokenizer, so that we will have to update the `bert-base-dutch-cased-config.json` file on AWS (Best option in my opinion).
- `distilbert-base-cased` and `distilbert-base-cased-distilled-squad` have hard coded `bos_token_id` in their config.json file on AWS (I checked), but the distilbert tokenizer doesn`t even have it -> is that correct? @VictorSanh 

#### TODO: 

- [x] Is the approach good for you? @thomwolf @julien-c @LysandreJik @mfuntowicz @sshleifer 
- [ ] Should we also check all community models whether their tokenizer differs from the default one? 
- [ ] I think the test I wrote is quite useful, but it uses Config and Tokenizer Classes in the same file, which is not in line with the current test files, which is why I didn't add it. Should we add a test like this? If yes, how? 




Comments:
Is good to merge for me if you guys are fine with it. Can open a new PR for a test proposal for this one and then also adapt the dutch bert model config on AWS. 

Isn't this breaking other hosted configs than the ones you're listing?

Like https://huggingface.co/microsoft/DialoGPT-large for instance? (more generally, we don't control which configs users use the lib with, so adding keys is fine, but renaming keys – or worse, changing their types – is costly)

PS: I do agree that having a `eos_token_id` is cleaner than having `eos_token_ids`

> Isn't this breaking other hosted configs than the ones you're listing?
> 
> Like https://huggingface.co/microsoft/DialoGPT-large for instance? (more generally, we don't control which configs users use the lib with, so adding keys is fine, but renaming keys – or worse, changing their types – is costly)

That's a very good point! There is one scenario, where it could break other hosted configs:

- The user defined an `eos_token_ids` that is different from the default `eos_token_id` the model has. 

But most of the time (just from browsing through some `config.json` files) the model was trained with HF and then saved and uploaded which means that the `eos_token_ids` was saved and is included in the `config.json`. In this case the values are the same as is the case for `https://huggingface.co/microsoft/DialoGPT-large`. In this case we have still have a dead parameter in the config which should be removed. 

I propose the following: 
I can write a script that checks the following for each configs:

1) does eos_token_ids exist ?
2) is eos_token_ids == default config.eos_token_id ?

If there are a lot of 1) and 2) then would write a bash script that simply replaces the line "`eos_token_ids` = [ ... ]" with `eos_token_ids`=...

Will report the results here

If you need an exhaustive list of all hosted models (with their config + files), you can do
```python
api = HfApi()
models = api.model_list()
```



Okey here is my analysis of the 308 (not bad actually! ) added community models:

1. **66** can't load either their config (n)or their tokenizer (including 3 facebook bart models because we call them directly by `bart-large-cnn` and not by `facebook/bart-large-cnn` -> should maybe add a new link or change model name online)
 2.  **79** currently have wrong `pad_token_id`, `eos_token_id`, `bos_token_id` in their configs. IMPORTANT: The reason for this is that we used to have the wrong defaults saved in `PretrainedConfig()` - see e.g. [here](https://github.com/huggingface/transformers/pull/2885/commits/77d958ac7f0b008df17656e3652246f602aef095)
the default value for **any** model for `pad_token_id` was 0. People trained a model with the lib, saved it and the resulting config.json now had a `pad_token_id = 0` saved. This was then uploaded. But it's wrong and should be corrected regardless of this PR. 
3. For **68** after changing `eos_token_ids` to `eos_token_id` we will have to remove the `eos_token_ids` parameter and possibly adapt the `eos_token_id` parameter - almost all of which we have to change anyway (1 exception) 
4. For **162** models everything is fine! 

Here the full analysis log [here](https://github.com/patrickvonplaten/files_to_link_to/blob/master/results.txt)
Here the code that created this log (simple comparison of loaded tokenizer and config with default config): [here](https://github.com/patrickvonplaten/files_to_link_to/blob/master/test_all_community_models.py)
Here the 308 models I checked: [here](https://github.com/patrickvonplaten/files_to_link_to/blob/master/all_community_models.txt)

**First conclusion:** 
- I think we can merge this PR as all models for which this PR would change to a "wrong" behavior already have a "wrong" behavior that should be fixed. The sooner we merge the sooner we have the correct API. 

**Second conclusion:** 
I think besides the FB models 1) is not really our job to fix. 
But 2) and 3) I think should be fixed on AWS. I'm happy to do this using some automated bash/python scripting. I would try out that it work on 1,2 community models and then apply it to all other cases (to not screw something up on AWS). 

Would that be good for you @julien-c @thomwolf @LysandreJik ?

In a future PR we could think about some automated testing that tokenizer configs are equal to model configs. 

> If you need an exhaustive list of all hosted models (with their config + files), you can do
> 
> ```python
> api = HfApi()
> models = api.model_list()
> ```

Great, this will be very helpful when writing an automated script to corret the config.json of the community models! 

# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/3264?src=pr&el=h1) Report
> Merging [#3264](https://codecov.io/gh/huggingface/transformers/pull/3264?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/8becb732931bbab5dd75cca5f5e7c75b2516d10b&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `97.91%`.

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

```diff
@@            Coverage Diff             @@
##           master    #3264      +/-   ##
==========================================
- Coverage   77.64%   77.55%   -0.10%     
==========================================
  Files         100      100              
  Lines       16979    16970       -9     
==========================================
- Hits        13184    13161      -23     
- Misses       3795     3809      +14     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/3264?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/\_\_init\_\_.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9fX2luaXRfXy5weQ==) | `98.92% <ø> (ø)` | |
| [src/transformers/configuration\_bart.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX2JhcnQucHk=) | `100.00% <ø> (ø)` | |
| [src/transformers/configuration\_t5.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX3Q1LnB5) | `96.55% <ø> (ø)` | |
| [src/transformers/modeling\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ191dGlscy5weQ==) | `91.58% <94.11%> (+0.21%)` | :arrow_up: |
| [src/transformers/configuration\_albert.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX2FsYmVydC5weQ==) | `100.00% <100.00%> (ø)` | |
| [src/transformers/configuration\_bert.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX2JlcnQucHk=) | `100.00% <100.00%> (ø)` | |
| [src/transformers/configuration\_distilbert.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX2Rpc3RpbGJlcnQucHk=) | `100.00% <100.00%> (ø)` | |
| [src/transformers/configuration\_flaubert.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX2ZsYXViZXJ0LnB5) | `100.00% <100.00%> (ø)` | |
| [src/transformers/configuration\_gpt2.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX2dwdDIucHk=) | `97.29% <100.00%> (ø)` | |
| [src/transformers/configuration\_roberta.py](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX3JvYmVydGEucHk=) | `100.00% <100.00%> (ø)` | |
| ... and [7 more](https://codecov.io/gh/huggingface/transformers/pull/3264/diff?src=pr&el=tree-more) | |

------

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


More in-detail analysis why **67** (actually one more now) models can't be loaded - log files are updated and use `api = HfApi(); models = api.model_list()` now. 

A) **34** models can't even load their config file. The reasons for this are either: 

1. **11/34**: Model identifier is wrong, e.g. `albert-large` does not exist anymore, it seems like it was renamed to `albert-large-v1`. These models have saved the wrong name online that how it is saved on AWS. Can easily be corrected. *e.g.*
The model_identifier: b`ertabs-finetuned-xsum-extractive-abstractive-summarization` does not exist, but `remi/bertabs-finetuned-xsum-extractive-abstractive-summarization` does exist -> wrong model identifier. Just 11 cases, so easy to correct.

2. **23/34**: There is an unrecognized `model_type` in the config.json, `e.g.` 

> "Error: Message: Unrecognized model in hfl/rbtl3. Should have a `model_type` key in its config.json, or contain one of the following strings in its name: t5, distilbert, albert, camembert, xlm-roberta, bart, roberta, flaubert, bert, openai-gpt, gpt2, transfo-xl, xlnet, xlm, ctrl
> "

Here I think we should add a `model_type` to the config (probably `bert` most of the time)

B) **33** models can load their config, but cannot load their tokenizers. The error message is almost always the same **32/33**: 

> TOK ERROR: clue/roberta_chinese_base tokenizer can not be loaded
> Message: Model name 'clue/roberta_chinese_base' was not found in tokenizers model name list (roberta-base, roberta-large, roberta-large-mnli, distilroberta-base, roberta-base-openai-detector, roberta-larg
> e-openai-detector). We assumed 'clue/roberta_chinese_base' was a path, a model identifier, or url to a directory containing vocabulary files named ['vocab.json', 'merges.txt'] but couldn't find such vocab
> ulary files at this path or url.

Here: the model has neither of: 

- `vocab_file`
- `added_tokens_file`
- `special_tokens_map_file`
- `tokenizer_config_file`

-> So we would have to upload one of those files to make it work. Not sure how time-consuming this is!

and we got one tokenizer which does not even have a path: `Message: stat: path should be string, bytes, os.PathLike or integer, not NoneType`

So I think it's mostly just renaming the model identifiers to their correct names and adding some tokenizer names.

@julien-c