Attempt to make load `from_tf=True` more efficiently
Author: Narsil
Pull request: yes

# What does this PR do?

- Remove the need to get 2X RAM when loading from h5 file
- Remove the need to have TF present to load `from_tf=True` (This is reduced to h5 only)

A much better version (imo) of the finale code resides here:

https://github.com/Narsil/transformers/pull/4

Keeping separate PRs since it's a more involved change.


Script:
```python
from transformers import GPT2Model

model = GPT2Model.from_pretrained("gpt2", from_tf=True)
```

Making sure network is not a thing:
```
TRANSFORMERS_OFFLINE=1 /usr/bin/time -v python test.py
```

Master:
```
	Command being timed: "python test.py"
	User time (seconds): 9.90
	System time (seconds): 2.24
	Percent of CPU this job got: 224%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:05.42
	Maximum resident set size (kbytes): 2411224
```

With this PR:
```
	Command being timed: "python test.py"
	User time (seconds): 2.41
	System time (seconds): 0.97
	Percent of CPU this job got: 161%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:02.09
	Maximum resident set size (kbytes): 1096128
```

And just a comparison point loading from Pickled file directly:
```
	Command being timed: "python test.py"
	User time (seconds): 2.63
	System time (seconds): 0.95
	Percent of CPU this job got: 178%
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:02.00
	Maximum resident set size (kbytes): 1319596
```

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->

Fixes # (issue)


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [ ] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- albert, bert, xlm: @LysandreJik
- blenderbot, bart, marian, pegasus, encoderdecoder,  t5: @patrickvonplaten, @patil-suraj
- longformer, reformer, transfoxl, xlnet: @patrickvonplaten
- fsmt: @stas00
- funnel: @sgugger
- gpt2: @patrickvonplaten, @LysandreJik
- rag: @patrickvonplaten, @lhoestq
- tensorflow: @LysandreJik

Library:

- benchmarks: @patrickvonplaten
- deepspeed: @stas00
- ray/raytune: @richardliaw, @amogkam
- text generation: @patrickvonplaten
- tokenizers: @n1t0, @LysandreJik
- trainer: @sgugger
- pipelines: @LysandreJik

Documentation: @sgugger

HF projects:

- datasets: [different repo](https://github.com/huggingface/datasets)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Examples:

- maintained examples (not research project or legacy): @sgugger, @patil-suraj
- research_projects/bert-loses-patience: @JetRunner
- research_projects/distillation: @VictorSanh

 -->

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_16173). All of your documentation changes will be reflected on that endpoint.

@sgugger @LysandreJik I think all the tests are passing now. Is there any thing I should be extra careful about with this / test in some form ?

> I've left a few comments on the PR but I'm extremely nervous about changing a core piece of the library like this. This function is not intended to be performant as the conversion should be done once and for all, and then the TF weights should be uploaded in the repo for every other users.

Yes, I fully understand the nervousness, I don't claim to be 100% it won't break anything.
I think the pure optimization is NOT the only goal here (although it's the only tangible result in this PR).

The bigger picture is an alternative for `pickle` which is unsafe by default and is used throughout PT.
During the discussions around options to make things safer, the point was raised if it was even possible to load directly from h5 without the intermediary `tf_model`. Different tensor shapes was  mentioned (indeed convolutions use different tensor forms).

Fortunately, it seems that the naming and pre-existing code is so consistent that we can do the work directly without resorting to loading the thing in tensorflow.

This PR is the result of my exploration of the problem where I think I show that it's possible to purely use the pre-existing h5 files on models that support it.

The follow-up also shows (to the best of my knowledge) that we can create an h5 files purely from the pickled file (because all tensor manipulation happen from tensor name and don't require actual models to determine transformations), meaning that this option is actually viable if we want to propose a safer alternative to pickle in PT.

Besides security `h5` could also solve some issues encountered by @stas00 for very large model files, where here you can load each tensors individually, and with lower RAM requirements.
This PR enables to use even less RAM than the PT way because the tensors only exist in RAM one at a time before being fed onto the network (we can even remove that single copy should we want it).

I will remark this as Draft just because as you mentioned, it shouldn't be merged without the extra caution it should come with.
Should we even want to merge this.
(We could imagine doing a real sweep of model files on the hub to make sure that this works on the real data saved out there besides the tests for one, + running all slow tests too).



Also a bit nervous about this PR, but I see the gain we're getting with such a change! It's a good sign that all tests are passing here since we do test `from_tf=True` for all models. 

For me I think I'm generally fine with this I think. Could we maybe do the following:
- Run the changes for all pytorch and tensorflow tests cross tests (`RUN_PT_TF_CROSS_TESTS`). (No need to test the Slow tests), but we need to be sure that all cross tests are valid
- @Narsil could you specific why we need both a `apply_transpose` and a `apply_reverse` method? How is the tensor reshaping / transposing logic different from before? 


> Also a bit nervous about this PR, 

Would a solution like using the new code in a `try.. except` and running the exact old code if it fails make everyone feel less nervous ? (I know it would reduce my own nervousness)

> Run the changes for all pytorch and tensorflow tests cross tests (RUN_PT_TF_CROSS_TESTS). (No need to test the Slow tests), but we need to be sure that all cross tests are valid

That's already done I think. I am not sure why but I get some 5 skipped tests on brutasse, but otherwise it works, and seems to be checked by the ci already too here : 
https://app.circleci.com/pipelines/github/huggingface/transformers/36539/workflows/629c3ec0-d69f-44f2-bcec-256724806311/jobs/399239

> @Narsil could you specific why we need both a apply_transpose and a apply_reverse method? How is the tensor reshaping / transposing logic different from before?

In this PR, reshaping is very close to the same. The only thing I allowed myself to change here was to prevent logic reshape fails (Like imagine we have a `(ATTN, VOCAB_SIZE)` weight in TF, and the PT target expects `(VOCAB_SIZE, ATTN)`. Imagine the code messed up and does not declare the right `TransposeType`, then maybe we could end up doing `(ATTN, VOCAB_SIZE)reshape((VOCAB_SIZE, ATTN))` which would work technically but it's a bug IMO since tensors are different from expected. Looking at the failures when `reshape` was missing, I deduced that the code was meant to  simply handle (..., 1, ... 1 ) differences where it wouldn't necessarily be about `squeeze`/`unsqueeze` but also the position of the `1-dim` in the tensor.  Doing reshapes of that sort is perfectly safe, so I kept it as-is.

> In the follow-up (https://github.com/Narsil/transformers/pull/4) I remove this logic entirely by using new extra `TransposeType` instead. It is even scarier since TransposeType is defined purely through weight naming so if any arch doesn't have those equivalence tests, maybe we're changing tensors in an unintended way (it will crash and not be a silent error, but still).
> 
> Having `TransposeType` be invertible is IMO a necessary property to provide confidently some new functionality to save things in the `h5` instead of `pickle` with minimum changes.
> 
> ```python
> weight_name = "clip.model.conv.bias"
> tensors = torch.Tensor((x, y, z))
> 
> tf_name, transpose = transpose_type = convert_pt_weight_name_to_tf_weight_name(weight_name, tensor.shape) # Does not exist yet 
> 
> 
> # Save
> tf_tensor = apply_reverse(tensor, tranpose) 
> save_h5(tf_name, tf_tensor)
> ```
> The key thing here is that `transpose` needs only the name and `apply_reverse` would need only the original tensor, it wouldn't need to know the target.
> Reversing the name itself is a bit more challenging since it's destructive in the TF -> PT path (`prefix_to_remove`) but I haven't explored that part yet, maybe the naming is consistent enough that we can still invert the name too.
> 
> If we get there that means we could *always* save `tf_model.h5` purely from the PT model. (It does limit a little the TF naming of things, but it seems that naming is sooo consistent across the library that it's sort of already maintained/guaranteed at some point.

All of this, is to provide the least possible amount of modifications to support `h5`. If we added a new file like `pytorch_model.h5` then we can purely dictate what goes on here (but it does mean creating a split in the library serialization format, meaning people with older transformers wouldn't be able to load the new `pytorch_model.h5` files etc.. It would cause much confusion where it is used etc... Not ideal.
 
Keeping everything in already defined names means we can leverage pre-existing files and provide a safe/lower memory alternative on any model that has both PT and TF support, without modifying anything for users/models which is already quite nice !

> And **if** I get the naming thing reversible we add support to `save_pretrained(as_h5=True)` to allow saving directly in `h5` format (without needing to support the model in TF) and loading `(from_tf=True)` meaning we can also plan a smooth transition or make the choice of format a user-decision etc.. It's a big IF


A thing that would make me less nervous is splitting the refactoring and the actual code changes in two different PRs so we can see more clearly the actual diff.

On the second PR that does the code change, testing the code works well for all kinds of architectures with checkpoints on the Hub would also be reassuring (it's tested in the CI on small configs but adding an integration check would be safer). We could test all checkpoints that are in the mappings with urls for instance (at the beginning of each modeling file).

> On the second PR that does the code change, testing the code works well for all kinds of architectures with checkpoints on the Hub would also be reassuring

@sgugger good insight, I am finding many things which are not covered by the fast tests (mostly in the heads I think).

I am looking into it.

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.