[Don't merge yet][T5, Bart] Allow t5 torch trace
Author: patrickvonplaten
Pull request: yes

This PR would fix #5647 .

It's not a great solution IMO though. 
The problem with torch script is that one **cannot** pass keyword arguments, but has to pass positional arguments and it is not possible to pass `None` because every input is required to be a tensor.
Because T5 requires both `input_ids` and `decoder_input_ids`, the two arguments should arguably be placed as the first two arguments.

There might be use cases though, where the same error would occur, which we could not save then, *e.g.* one wants to input `input_embeds`.

Maybe @LysandreJik @sgugger have a better idea.

Comments:
Looks innocuous to me, is there some test this allows us to enable for jit and T5?

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

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

```diff
@@            Coverage Diff             @@
##           master    #6268      +/-   ##
==========================================
- Coverage   79.79%   78.50%   -1.29%     
==========================================
  Files         148      148              
  Lines       27196    27196              
==========================================
- Hits        21701    21351     -350     
- Misses       5495     5845     +350     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/6268?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_bart.py](https://codecov.io/gh/huggingface/transformers/pull/6268/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19iYXJ0LnB5) | `95.76% <ø> (ø)` | |
| [src/transformers/modeling\_t5.py](https://codecov.io/gh/huggingface/transformers/pull/6268/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190NS5weQ==) | `84.46% <ø> (+1.13%)` | :arrow_up: |
| [src/transformers/modeling\_tf\_mobilebert.py](https://codecov.io/gh/huggingface/transformers/pull/6268/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9tb2JpbGViZXJ0LnB5) | `24.55% <0.00%> (-70.09%)` | :arrow_down: |
| [src/transformers/tokenization\_t5.py](https://codecov.io/gh/huggingface/transformers/pull/6268/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fdDUucHk=) | `71.83% <0.00%> (-23.95%)` | :arrow_down: |
| [src/transformers/tokenization\_openai.py](https://codecov.io/gh/huggingface/transformers/pull/6268/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fb3BlbmFpLnB5) | `71.21% <0.00%> (-12.88%)` | :arrow_down: |
| [src/transformers/file\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/6268/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9maWxlX3V0aWxzLnB5) | `82.44% <0.00%> (+0.25%)` | :arrow_up: |
| [src/transformers/tokenization\_dpr.py](https://codecov.io/gh/huggingface/transformers/pull/6268/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fZHByLnB5) | `57.65% <0.00%> (+4.50%)` | :arrow_up: |
| [src/transformers/modeling\_tf\_flaubert.py](https://codecov.io/gh/huggingface/transformers/pull/6268/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9mbGF1YmVydC5weQ==) | `87.73% <0.00%> (+63.19%)` | :arrow_up: |

------

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


After digging a bit deeper why Bart tests fail, I think the reason is that the Bart cache/`past_key_value` data structure is not compatible with torchscript. For now the Bart tests pass because returning the cache/`past_key_vaule` is disabled - see https://github.com/huggingface/transformers/blob/ac001c48b8df1f5aadcc8cf2c71d7c1116c05250/tests/test_modeling_common.py#L252. 

A bug was filed for this problem: https://github.com/huggingface/transformers/issues/6348.

PR should be good for merge IMO. @LysandreJik @sshleifer @sgugger - would be great if you can take a quick second look.

Putting this on hold for now as it introduces a breaking change.

Any updates on this?

The problem is that it breaks backwards compatibility in a sense that the positional arguments of Bart and T5 are changed. At the moment this is the only option to make torch tracing work for Bart and T5 though...there might be a possiblity to trace a wrapper around the model though - see https://github.com/pytorch/pytorch/issues/14455 . But this currently leads to another problem which is probably related to our PyTorch models not being scriptable at the moment.