[WIP] Replace instances of  `config.output_hidden_states` with function argument  `output_hidden_states` in all possible models.
Author: drjosephliu
Pull request: yes

Attempts to close #3879 by refactoring `config.output_hidden_states` as an argument,`output_hidden_states`, to functions `forward()` and `call()`. Affects all PT and TF models that output hidden states.

Currently it is failing the following tests: `run_tests_tf`, `run_tests_torch` and `run_tests_torch_and_tf` because they are still using `config.output_hidden_states`. Please advise on how I should go about testing this. 

Comments:
Hey @drjosephliu - thanks so much for opening this PR! 
Sorry for being lazy here - could you check out the comments I added in PR #4538 - I think they apply 1-to-1 the same way here.

No problem. I'm a bit busy this week, but will aim to get it done by the end of the week.

> No problem. I'm a bit busy this week, but will aim to get it done by the end of the week.

Sure, take your time :-) 

I'm just in the middle of fixing up the tests and I've noticed that the `ReformerModel` has a different method signature than other models because it takes in `do_output_hidden_states`:

```
    def forward(
        self,
        input_ids=None,
        attention_mask=None,
        position_ids=None,
        head_mask=None,
        inputs_embeds=None,
        num_hashes=None,
        do_output_hidden_states=False,
        do_output_attentions=False,
    ):
```

Should I be converting it to `output_hidden_states` here?

> I'm just in the middle of fixing up the tests and I've noticed that the `ReformerModel` has a different method signature than other models because it takes in `do_output_hidden_states`:
> 
> ```
>     def forward(
>         self,
>         input_ids=None,
>         attention_mask=None,
>         position_ids=None,
>         head_mask=None,
>         inputs_embeds=None,
>         num_hashes=None,
>         do_output_hidden_states=False,
>         do_output_attentions=False,
>     ):
> ```
> 
> Should I be converting it to `output_hidden_states` here?

yes please!

Before starting on the TF implementation, it might be a good idea how it is handled in `modeling_tf_bert.py` of PR: #4538 :-) 

So all pytorch tests are passing, but I still haven't really figured out the TF ones. I copied what you did for `output_attention_heads` applied to `output_hidden_states` to TF Bert almost verbatim, but I'm still getting some failing tests

Hey @drjosephliu,

This looks great already :-) We will probably have a lot of conflicts with master once #4538 is merged (I should have thought about this before ... ). Would it be ok for you to wait a couple of days on this PR until we merge #4538. Then I can rebase this PR to master and it will be easier to work from then on :-) 

Can you do the following change to the branch `hidden_states` 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

If this doesn't work, maybe just send me a collaboration invite to @patrickvonplaten :-)

Looking forward to merge this soon :-) 

Hey, the checkbox "Allow edits by maintainer" is already checked - I guess I should leave it checked right?

Hey @drjosephliu,

I tried to rebase to main, but it's just not reasonable. We have to many merge conflicts with `output_attentions` (in every file in every forward function) and not just for one commit, but for 7 commits :-/ At this point it would be much faster to just open a new PR. 

I should have seen this coming, so that you guys would not have started at the same time :-/ 
Super sorry about that! 

There are two solutions:
1) You can open a new PR and sadly starting from scratch (Recommended):
If you look at the merged PR here: https://github.com/huggingface/transformers/pull/4538, you can see that we actually decided to keep `config.output_attentions` for now and only overwrite it with the arguments of the forward function. This means that the high-level forward functions all have the argument `output_hidden_states = None`. 

2) You can try to go through the whole rebase yourself (Not recommended):
- In your repo if you run `git rebase main/master` from this branch, you see a bunch of merge conflicts arising (and that's just for the first commit of this branch). You could try to solve them correctly one-by-one, but you have to be careful to not introduce new bugs here. So I strongly advise against this.

3) You don't feel like doing the same work again (Very much understandable :D). I would totally understand it, if you don't want to do the same work again. In this case I would re-open the issue to the community or do it myself. I would totally understand this - we have other "good first issue" tags.

I hope, you give it a try with 1) :-) Very sorry about the merge conflicts again - let me know what you think

Not a problem. I'm quite familiar with the codebase now so it shouldn't take too long.

Saw your new PR @drjosephliu ! Thanks a lot - it's great that you tackle this :-) I will take a look tomorrow at the new PR :-)