Skipping outputs
Author: LysandreJik
Pull request: yes

Currently, `encode_plus` and `batch_encode_plus` return the same outputs for different models.

This is sub-optimal as we can't do the following for each model:

```py
inputs = tokenizer.encode_plus(sequence, return_tensors="pt")
model(**inputs)
```

This will crash for DistilBERT as the tokenizer would return `token_type_ids` which can't be handled by the model.

In order to fix this, each tokenizer has to return model-specific arguments. Usually there are the same default arguments, and some models handle less (e.g. DistilBERT, RoBERTa).

This is a mock PR offering a solution using a ~`skip_outputs`~ `return_outputs` argument to tokenizers.

```py
from transformers import DistilBertTokenizer

tokenizer = DistilBertTokenizer.from_pretrained("distilbert-base-cased")
print(tokenizer.encode_plus("Hey, how are you?"))
```

Returns a dictionary without the token type ids:

```py
{'input_ids': [101, 4403, 117, 1293, 1132, 1128, 136, 102], 'attention_mask': [1, 1, 1, 1, 1, 1, 1, 1]}
```

Specifying a custom ~`skip_outputs`~ `return_outputs` at initialisation works as expected:

```py
from transformers import DistilBertTokenizer

tokenizer = DistilBertTokenizer.from_pretrained("distilbert-base-cased", return_outputs=["attention_mask", "token_type_ids"])
print(tokenizer.encode_plus("Hey, how are you?"))
```

```py
{'input_ids': [101, 4403, 117, 1293, 1132, 1128, 136, 102], 'token_type_ids': [0, 0, 0, 0, 0, 0, 0, 0], 'attention_mask': [1, 1, 1, 1, 1, 1, 1, 1]}
```

or with a custom ~skipped~ output:

```py
from transformers import DistilBertTokenizer

tokenizer = DistilBertTokenizer.from_pretrained("distilbert-base-cased", return_outputs=["token_type_ids"])
print(tokenizer.encode_plus("Hey, how are you?"))
```

```py
{'input_ids': [101, 4403, 117, 1293, 1132, 1128, 136, 102], 'token_type_ids': [0, 0, 0, 0, 0, 0, 0, 0]}
```

This also works with saving/reloading:

```py
from transformers import DistilBertTokenizer

tokenizer = DistilBertTokenizer.from_pretrained("distilbert-base-cased", return_outputs=["token_type_ids"])
print(tokenizer.encode_plus("Hey, how are you?"))
tokenizer.save_pretrained("xxx")

tokenizer = DistilBertTokenizer.from_pretrained("xxx")
print(tokenizer.encode_plus("Hey, how are you?"))
```

Returns the following:

```py
{'input_ids': [101, 4403, 117, 1293, 1132, 1128, 136, 102], 'token_type_ids': [0, 0, 0, 0, 0, 0, 0, 0]}
{'input_ids': [101, 4403, 117, 1293, 1132, 1128, 136, 102], 'token_type_ids': [0, 0, 0, 0, 0, 0, 0, 0]}
```

Comments:
Nice.

One question: do we want to have a `skip_output` flag  or to have a `keep_output` flag.

`skip_output` seems to me as introducing a dependency to be maintained between all the models (if we add a model with additional output that are processed by encode_plus later, we would have to update all the models to avoid this output)

`keep_output` is longer to write right now (we have to add it for all the models) but once it's added, all the models are independent from each others.

I'm ok with both solutions (by the way, in general terms, a lot of software can accept a combination of whitelist and/or blacklist. When both are present, it's usually "include the whitelist, and remove the blacklist")

If we do `keep_output`, maybe we name the attribute `return_outputs: List[str]` for consistency with `encode_xxx()` params?

I agree with both of you. Furthermore, this approach (deleting from the dict `encode_plus` generated) is not compatible with the `return_xxx` in the `encode_plus` arguments.

I'm implementing both your proposed changes, looking into fixing the above and into fast tokenizers.

I'll then move on to the tests.

- [x]  replace the blacklist by a whitelist
- [x] rename to `return_outputs` for consistency with `encode_plus arguments`
- [x] compatibility with all of `encode_plus`'s arguments
- [x] fast tokenizers
- [x] tests

I like the solution, 👍 .

One question: It requires the user to know / look at the names of the parameters handled by `__call__()` / `forward()`, should we expose a property on PreTrainedModel to give the list of parameter supported by the model ? This one will be overrided in Roberta and Distil.

```python
model = SomeModel(...)
tokenizer = AutoTokenizer.from_pretrained(..., return_outputs=model.input_names)
``` 

Indeed, such an attribute would be helpful! I'll add it and move on to the tests.

Regarding the suggestion of @mfuntowicz, in the end, this should be in a common configuration for model and tokenizers I guess, so maybe we could actually have this attribute as input to `PretrainedTokenizer.__init__()` already (instead of class attribute) to prepare for the future.

That value is currently managed by the `__init__` method, see the examples above

It still needs to be a class attribute in my opinion, as it should be overridden by children of `PreTrainedTokenizer` and it should be known by `encode_plus`/`encode`/`batch_encode_plus`.

Should be good for review. I reverted the `docs` commit because it made the review harder. I'll recommit the docs at merge time.

# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/3116?src=pr&el=h1) Report
> Merging [#3116](https://codecov.io/gh/huggingface/transformers/pull/3116?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/49debe62fdc96e161f866dd8914d5915477bb742?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/3116/graphs/tree.svg?width=650&token=9qOlN6Hb1c&height=150&src=pr)](https://codecov.io/gh/huggingface/transformers/pull/3116?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #3116      +/-   ##
==========================================
+ Coverage   77.98%   77.99%   +<.01%     
==========================================
  Files          98       98              
  Lines       16645    16660      +15     
==========================================
+ Hits        12981    12994      +13     
- Misses       3664     3666       +2
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/3116?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/tokenization\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/3116/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fdXRpbHMucHk=) | `91.85% <100%> (+0.12%)` | :arrow_up: |
| [src/transformers/tokenization\_roberta.py](https://codecov.io/gh/huggingface/transformers/pull/3116/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fcm9iZXJ0YS5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [src/transformers/tokenization\_distilbert.py](https://codecov.io/gh/huggingface/transformers/pull/3116/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fZGlzdGlsYmVydC5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [src/transformers/file\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/3116/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9maWxlX3V0aWxzLnB5) | `68% <0%> (-0.41%)` | :arrow_down: |
| [src/transformers/modeling\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/3116/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ191dGlscy5weQ==) | `94.4% <0%> (-0.16%)` | :arrow_down: |

------

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


Merged after offline review from @thomwolf and @julien-c