Add TFViTModel
Author: ydshieh
Pull request: yes

# What does this PR do?

Add `TFViTModel`. This is also a prerequisite task before we can add `TFVisionEncoderDecoderModel` as a continuation of `FlaxVisionEncoderDecoderModel` (#13359)

## Who can review?


Comments:
Looking forward to it @ydshieh! Let us know if you need anything to push this to the finish line.

Hi, @LysandreJik, just an update:

Most of the necessary work is done, including the test.

I did have a problem with the input format between `torch.nn.Conv2d` and `tf.keras.layers.Conv2D` (channel first/last), which leads to some weight loading (PT->TF) issue.

But I found the usage of `tf.nn.conv2d` in `TFDebertaV2`, which can fix the issue I think. If everything goes well, we will have vision for HuggingFace TF's models soon :)

Hi, @LysandreJik , @patrickvonplaten , @Rocketknight1 , @NielsRogge 

This PR is ready for review. I have run all the tests locally (including slow ones). The conversion `PT->TF` works well :)
(**Update**: there are still 2 slow tests in `test_modeling_tf_common.py` to be fixed)

One thing to keep in mind: This method

https://github.com/huggingface/transformers/blob/c9fbf7dc1969d96dcf7bf550b70b7873a8676207/src/transformers/models/vit/modeling_tf_vit.py#L88

uses `tf.image.resize`'s `bicubic` method, which has a hard-coded constant `alpah=-0.5` in TensorFlow,  but `-0.75` in `torch.nn.functional.interpolate`. Therefore, for images with size different from the config.image_size, we won't get the same results between PT/TF versions.

[References]
https://pytorch.org/docs/stable/_modules/torch/nn/functional.html#interpolate

https://github.com/tensorflow/tensorflow/blob/a214ba286151b3d6347a67302b1513360f1f727e/tensorflow/core/kernels/image/resize_bicubic_op.cc#L54


@ydshieh  Hey! Overall I like the code, and this is a great PR. I did encounter one issue in testing, though - I ran a notebook to compare output values between the TF and PT models and I found quite a large divergence in some output values, although the mean divergence was small. However, even a relatively generous test like `np.allclose(tf_final_hidden_out, torch_final_hidden_out, rtol=1e-01, atol=1e-02)` failed for me. I made a Colab with the tests I ran [here](https://colab.research.google.com/drive/1D-LFj27WH7TzpEnFBhN_ywxfpnr4RpRp?usp=sharing). Is it possible there are small differences in one of the layer implementations that could cause this?

The good news, though, is that this is the only major problem I could find. Saving and loading the model worked perfectly, and the outputs were identical, which means we should have no problems making TF checkpoints once this is resolved.

@Rocketknight1 Hi, ok, I will check, thanks. Sorry about this, I thought I have run necessary tests (pt ->tf), but apparently there is something wrong.

@ydshieh Don't worry, my PRs usually have way worse problems than this, lol

Hi, @Rocketknight1 ,

(Forget my previous message, I made some stupid statements ...)

I run your test script locally on my laptop, but I always get the same results, like

```
True
0.0003039837
1.541685e-06
0.6905535
```

Currently I am not sure where the issue is - I will try to run on a Linux VM.

On a Linux VM, still no issue. Do you test with GPU? I don't have GPU on my laptop, so the tests are all on CPU.
I can try to run the tests on Colab GPU.


@ydshieh I was testing with GPU, yes. I tried with CPU and the problem disappeared, so this seems to be GPU-specific.

Actually, let me investigate a little more - I just realized I was running the torch model on CPU. Maybe this is just a CPU - GPU difference!

> 
> 
> Actually, let me investigate a little more - I just realized I was running the torch model on CPU. Maybe this is just a CPU - GPU difference!

No problem. Thank you :)

Yes, I confirmed I'm seeing significant differences between the output on CPU and GPU for Torch. Most likely the problem is just caused by CPU/GPU differences instead of Torch/TF differences. I'll try to set up a GPU environment with both GPU-enabled Torch and TF tomorrow and confirm that the results are close as expected. Sorry for worrying you!

I remembered seeing something in `torch.nn.Conv2d` like

```
In some circumstances when given tensors on a CUDA device and using CuDNN,
this operator may select a nondeterministic algorithm to increase performance.
If this is undesirable, you can try to make the operation deterministic
(potentially at a performance cost) by setting `torch.backends.cudnn.deterministic = True.`
See Reproducibility for more information.
```

Maybe this is somehow related. Once we identify the cause, it would be a good idea to put this knowledge somewhere in `transformers` :)

See
https://pytorch.org/docs/stable/generated/torch.nn.Conv2d.html

I finished testing and there's still a similar difference between the torch-GPU outputs and the TF-GPU outputs, but, like you said, that could possibly be caused by using different algorithms. I tried playing around with `torch.cudnn.benchmark` but the problem persisted.

I did confirm that torch + CPU and TF + CPU give identical outputs - this means your code is very likely bug-free. I'll ask around and see if anyone on the team has any suggestions, but maybe we should just merge as-is.

@ydshieh If you want, you can try to investigate to find the single op that has differing outputs between the two libraries. However, I don't think it's totally necessary since the outputs are mostly very close, so if you're not interested we can probably merge as-is. Up to you!

@Rocketknight1 , thank you for the effort!

Let's wait a bit before merging - I would like to see if I am able to avoid the transpose in the call method first.
Is this OK for you?

About the GPU result difference, I think I won't look at it at this moment.
(One of the main reason is that it's kind difficult for me - I don't have any GPU machine. Working on Colab GPU for debugging this is not very handy.)

That's perfect, yes. Thanks for your contributions, there's no rush with finishing this PR!

@Rocketknight1 I have successfully made conv2D weight loading PT <-> TF using their own native shape, and therefore there is no need to transpose the conv. kernel in `TFPatchEmbeddings`.

If this new change is OK for Hugging Face, I will clean up this block

https://github.com/huggingface/transformers/blob/69900345c668defbb34c9071eee6c90d722c7e6e/src/transformers/models/vit/modeling_tf_vit.py#L202-L205

It might be a good idea for a second person to double check the change added to `modeling_tf_pytorch_utils.py`.

Thanks :)


Well, let me try if I can use a `tf.keras.layers.Conv2D` with `use_bias=True` directly and still make the weight loading work :) 

Use tf.keras.layers.Conv2D instead of tf.nn.conv2d, and it works well with PT/TF weight loading.

@ydshieh Taking a look now! That change to `modeling_tf_pytorch_utils.py` has generated some lively discussion in the Hugging Face Slack, lol.

In general, everyone agrees that we shouldn't need to do a `transpose()` in the `call()` method, so we need some way of transposing the weights at convert time, but `_get_tf_conv2d_kernel_names` might cause problems. Right now we have a heuristic like [this](https://github.com/huggingface/transformers/blob/2024faf1713272464336ad97f371787b24cb3c53/src/transformers/modeling_tf_pytorch_utils.py#L60) which we use for transposing certain weights already. Could we possibly just alter that heuristic slightly? 

@sgugger suggests that function could detect convolution weights by their name/rank (i.e. that they are 4-dimensional tensors and contain 'kernel' in their name). Then we could return an enum from that function (NO / SIMPLE / CONV), and then use that enum in `load_pytorch_weights_in_tf2_model` to decide which weights to transpose. What do you think?

Hi @Rocketknight1 

Sound like a good way, I will try it :)
(A bit overthinking previously)

@ydshieh I agree that the heuristic feels a bit dangerous, but we have plenty of tests, so we should notice if it causes any problems!

Hi, I made the suggested change. It is not completely the same as the suggestion, because

https://github.com/huggingface/transformers/blob/00c24707b54264fd2d788802d19681eda8e539cc/src/transformers/modeling_tf_pytorch_utils.py#L30

only get `tf_name` without the weight array, so I am not able to determine if a kernel is a Conv2D by checking the shape inside this method. The check is done inside:

in `load_tf2_weights_in_pytorch_model`
https://github.com/huggingface/transformers/blob/00c24707b54264fd2d788802d19681eda8e539cc/src/transformers/modeling_tf_pytorch_utils.py#L189 and
in `load_tf2_weights_in_pytorch_model`
https://github.com/huggingface/transformers/blob/00c24707b54264fd2d788802d19681eda8e539cc/src/transformers/modeling_tf_pytorch_utils.py#L365

(However, we loss some tf weight name processing that are done inside `convert_tf_weight_name_to_pt_weight_name`)

The test `TFViTModelTest::test_pt_tf_model_equivalence` passed, so I think it works fine :)


You raise a good point @ydshieh I think it might be cleaner if we add the array as an argument to `convert_tf_weight_name_to_pt_weight_name` to only do once the right transposition instead of combining two different ones. This would be more readable, thus more bulletproof.

@sgugger It looks like `convert_tf_weight_name_to_pt_weight_name` is public, I am afraid to change it.

If Hugging Face would accept a change to this method - making a new required argument, I can do it in this PR.
(making this argument optional is not very logical, I guess). 

It's meant to be internal, but if afraid of breaking anything, you can make that new argument optional and only use it if it's not None.

Well, I just checked after my message, and it is only used in 2 methods about weight loading :). I will change it.

I moved the logic of conv2D layer detection into `convert_tf_weight_name_to_pt_weight_name`.
(I found the same check is done for flax <-> pt)

Hi, is the latest change about Conv2D/Transpose in `modeling_tf_pytorch_utils.py` OK?

Let me know if there is anything to address in this PR, thank you.

Failing test is unrelated. Like the new design a lot! Thanks a lot for all the work!

Let me fix some (minor, I think) issues after applying the suggestions.

Failed tests are unrelated. Is it necessary or preferable to rebase onto master?

Thank you all for the efforts reviewing this PR!

P.S. I am curious about why new commits could have more unrelated failed tests than previous commits - would be great if some of you can have a short explanation to me (for learning purpose), thanks.

Hi @ydshieh, I believe most of the errors here are unrelated to your PR: they're related to a recent change in `datasets`. For audio datasets, it now takes care of the audio decoding, but means that you'll need to have `librosa` installed in order to do so. 

We updated our tests to reflect this change, but since your PR has not been rebased on `master` for a while, it doesn't use the updated code. Rebasing on `master` would probably clear all of these issues :)

@LysandreJik Thanks!

After rebasing on master, there is an issue I need to address (I think this is a recent test @patrickvonplaten added).

```
FAILED tests/test_modeling_vit.py::ViTModelTest::test_pt_tf_model_equivalence
```

@patrickvonplaten Would be great if you can check (when you have time) the modification done in `test_modeling_common.py` in https://github.com/huggingface/transformers/pull/13778/commits/0e546f6b1e2de5877ffd5c84e71d8d4de178d440

This is necessary to fix the issue appeared previously
(https://app.circleci.com/pipelines/github/huggingface/transformers/29656/workflows/d083d4db-6012-47af-b741-51f792b5a892/jobs/298830)

```
>   ???
E   tensorflow.python.framework.errors_impl.InvalidArgumentError: cannot compute Conv2D as input #1(zero-based) was expected to be a int32 tensor but is a float tensor [Op:Conv2D]
```

After rebase on master, it's green again :-)

Thanks again for all your work on this!

@sgugger There is no tf checkpoint for ViT. The test uses from_pt=True, but usage examples in model script don't specify this currently.

I can upload a checkpoint and make necessary changes in another PR. But maybe it's better to have TF checkpoint under HF rather than under my repo.

Sorry about this.

We will update TF checkpoints in the repos, then you can make another PR to adapt the tests :-)