[All models] Extend config.output_attentions with output_attentions function arguments
Author: bharatr21
Pull request: yes

Attempts to close #3880 by refactoring `config.output_attentions` => `output_attentions` in `forward()`/`call()` functions

**UPDATE** from Patrick

@thomwolf @LysandreJik 
This PR adds the argument `output_attentions` to every forward function for more flexibility. This PR makes it possible to easily switch the output attention on/off without having to instantiate a new model every time.

The logic is the following. If 'output_attentions` is configured in the `forwrd()` fn => use that. If not => use `config.output_attentions`.

IMPORTANT: This PR does **not** change backward compatibility since output_attentions can still be configured using the config.

TESTS:
An additional test is added to the `test_output_attentions()` common test.

FUTURE PR:
- [ ] Clean the documentation. We still need to add this argument to the docs of all models and make sure the docs are clean. Lemme know @Bharat123rox if you want to tackle this in a new PR or if I should do it :-) It's not the most interesting PR, so I fully understand if you don't feel like doing it anymore ;-) 

Comments:
Also removed the tests for `output_attentions` since all of them were fetching values from the config

@Bharat123rox - that's awesome, thanks a lot! It's a lot of manual work, but it will be super useful once it's merged :-)

I added a bunch of comments - let me know if something is unclear.

Regarding the tests, let's try to first make all torch tests pass and then check the TF tests. 

Regarding the torch tests:

Most tests that fail are those where you removed `config.output_attentions` in the test, but didn't set `output_attentions=True` for the forward call. These tests previously were outputting the attentions but don't do this anymore. You should fix these tests if you set `output_attentions=True` in the forward pass. 

Looking forward to have this merged soon :-) Let me know if something is unclear!

 Now, most of the tests are giving new `AssertionError`

OK! Let's maybe try first to fix all the test `test_attention_outputs` tests. You can run this test for a specific model using the following command: 
```
pytest tests/test_modeling_openai.py::OpenAIGPTModelTest::test_attention_outputs
```
I fixed this test as an example for `openai` on this commit: https://github.com/huggingface/transformers/pull/4597/commits/e8efd72fce1be304043863cbab4cd7a61a39e434

It's gonna be a longer process to fix all tests. Let's try to start with the `test_attention_outputs` tests for all PyTorch models :-) 

Btw, can you do the following change to the branch `outputattentions` of your fork, so that I can commit directly to your branch? :-) 

https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork

https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork was not clear as I've already checked the "Allow edits by maintainers" option, however I have sent a collaboration invite to you @patrickvonplaten which I think should give enough permissions

@patrickvonplaten @thomwolf please help me in fixing the remaining PyTorch and TensorFlow test failures, they are of various kinds, mostly `AssertionErrors`

This is really great work! I will take a look at the failing tests :-) It would be great if you can rename some of the variables as mentioned above.

Ok great, I can take a look into the remaining tests now :-) 

> Ok great, I can take a look into the remaining tests now :-)

Yes, please do, thank you! there are only 3 Assertion failures in Torch and hopefully all failures in TF are also similar 🤞 

Hey @Bharat123rox,

I think from the PyTorch side we are ready now :-) 
Regarding the TF side, my last commit shows how to implement it for TF Models - could you give it a try for the other models? You always have to pay attention to points 1) - 3) as mentioned above to make sure that the TF Models can be trained, complised and serialized with keras.

Be careful to make a `git fetch` and `git pull` on your branch now before continuing to work since I added two commits. 

Let me know if you have any questions! :-) 
Really great work so far, I think we are almost finished :-) 

@patrickvonplaten Most of the TF tests are fixed, and the remaining seem to be different `AssertionErrors`, please help with the remaining TF test failures

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

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

```diff
@@            Coverage Diff             @@
##           master    #4538      +/-   ##
==========================================
+ Coverage   74.52%   75.91%   +1.39%     
==========================================
  Files         128      128              
  Lines       21497    21515      +18     
==========================================
+ Hits        16021    16334     +313     
+ Misses       5476     5181     -295     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/4538?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_mmbt.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19tbWJ0LnB5) | `22.11% <ø> (ø)` | |
| [src/transformers/modeling\_roberta.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19yb2JlcnRhLnB5) | `94.73% <ø> (ø)` | |
| [src/transformers/modeling\_tf\_flaubert.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9mbGF1YmVydC5weQ==) | `25.65% <0.00%> (ø)` | |
| [src/transformers/modeling\_tf\_roberta.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9yb2JlcnRhLnB5) | `74.74% <ø> (ø)` | |
| [src/transformers/modeling\_tf\_xlm.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl94bG0ucHk=) | `75.85% <73.33%> (-0.26%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_albert.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9hbGJlcnQucHk=) | `75.33% <75.00%> (-0.06%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_xlnet.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl94bG5ldC5weQ==) | `80.30% <76.19%> (-0.24%)` | :arrow_down: |
| [src/transformers/modeling\_flaubert.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19mbGF1YmVydC5weQ==) | `84.00% <80.00%> (+0.12%)` | :arrow_up: |
| [src/transformers/modeling\_tf\_t5.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl90NS5weQ==) | `93.71% <86.66%> (+0.01%)` | :arrow_up: |
| [src/transformers/modeling\_tf\_openai.py](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9vcGVuYWkucHk=) | `95.57% <88.88%> (-0.31%)` | :arrow_down: |
| ... and [33 more](https://codecov.io/gh/huggingface/transformers/pull/4538/diff?src=pr&el=tree-more) | |

------

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


I got an email with a comment from @patrickvonplaten  (which I can't find) expressing the opinion that 
>IMO use_cache, output_attentions and output_hidden_states all are not really parameters of the model or its decoding strategy, but switches for all (or some for use_cache)models that can be turned on and off, but essentially don't influence the model's output logits. In contrast the other config attributes ("max_length", "do_sample", ...) do influence the output of the model and therefore should stay in the config. Therefore I would be ok with removing use_cache, output_attentions and output_hidden_states completely from the config.

And I completely agree with that conclusion! We should make sure to highlight it in release notes.

> I got an email with a comment from @patrickvonplaten (which I can't find) expressing the opinion that
> 
> > IMO use_cache, output_attentions and output_hidden_states all are not really parameters of the model or its decoding strategy, but switches for all (or some for use_cache)models that can be turned on and off, but essentially don't influence the model's output logits. In contrast the other config attributes ("max_length", "do_sample", ...) do influence the output of the model and therefore should stay in the config. Therefore I would be ok with removing use_cache, output_attentions and output_hidden_states completely from the config.
> 
> And I completely agree with that conclusion! We should make sure to highlight it in release notes.

@sshleifer 
I just deleted this comment :D I rethought this a bit. I think the better solution is what we have now: Have `output_attentions` in both the config and as a forward argument. This way we can still use the keras serialize function, don't break backward compatibility and have the same logic as we do in the generate() method. 

@Bharat123rox - thanks a million for your work here! It was a lot of manual work in a lot of files! This PR is VERY useful for the library!