Tapas tf
Author: kamalkraj
Pull request: yes

# What does this PR do?

TF Tapas

<!--
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

 -->
@nielsrogge  @sgugger @LysandreJik @Rocketknight1

Comments:
![Screenshot 2021-09-02 at 9 11 41 PM](https://user-images.githubusercontent.com/17096858/131876382-eb7f11cd-7347-43ad-b478-7ed21d511c42.png)



pending

- [x] adding unit test cases
- [x] adding model to TFAutoTableQuestionAnswering pipeline
- [x] adding unit test for the pipeline
- [x] fix all the copy pasted code comments from pt->tf
- [x] update `tapas.rst` with TF sample code
- [x] add `#Copied` Comments`
- [x] push tf weights to official model hub - help needed.
- [x] update MLM model, https://github.com/huggingface/transformers/pull/13408
- [ ] ...

@LysandreJik 
ready for review 🤗

Great news @kamalkraj!

@NielsRogge and @Rocketknight1, could you take a look at this?

Hi, I'd like to apologize for not getting to this sooner! It's a huge PR, but I'll try to get through it today or tomorrow and give feedback where I can.

@LysandreJik @sgugger @Rocketknight1 @NielsRogge 
Thanks for the review.
Done changes according to review. 

The only pending part is `pushing TF weights to official model hub` -
After that, I can remove the model loading `from_pt` argument from all the tests

Hi,

While restoring model weights from `tf_model.h5` something is going wrong. 
Could you please check this notebook and confirm I am using correct save and load methods?

https://colab.research.google.com/drive/1GypOH9_70xhMCZvZhE0RSCWtx4r_RIzg?usp=sharing

@NielsRogge @Rocketknight1 

I can work on the PyTorch change first and submit a PR. 

I investigated the notebook and something very odd is happening - the output logits being masked (with addition of -10000) are not the same in the two models. I can't understand how a change in weights would cause that, so I'm guessing there's some deeper cause. I'll need to step through the model execution to figure out where that bug is creeping in, though.

@Rocketknight1 

https://github.com/huggingface/transformers/blob/408b2d2bd08f667cf4154730cc323c4e49657eed/src/transformers/modeling_tf_utils.py#L497-L514

Only the saving model layers are getting restored. 
![Screenshot 2021-10-13 at 7 06 50 PM](https://user-images.githubusercontent.com/17096858/137143492-ac4a1797-aecf-404c-a51b-4bac33c5f001.png)

Other parameters are not listed in the `tf_model.h5` file also


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/master/CONTRIBUTING.md) are likely to be ignored.

@Rocketknight1 could you take a look at this?

Hey @kamalkraj, I took a look at the issue. From what I'm understanding, the issue comes from the `compute_token_logits` method: using your notebook, I have compared the outputs of two models, one being saved and reloaded. I have verified that all the hidden states remain the exact same across the two model instances. From what I'm perceiving, the divergence appears in that `compute_token_logits` method.

In that method, you're doing a `tf.einsum` operation, using the `self.column_output_weights` and `self.column_output_bias` weights. However, it seems that when saving the model, these two weights are not saved. I identified this as when reloading the model weights from the TensorFlow checkpoint, the bias will be set to `0` as it is initialized from the default `tf.zeros_initializer`, rather than from the layer that we have just saved.

From what I have seen, this affects only the question answering head.

Is there a reason you used the `tf.einsum` approach instead of the `tf.EinsumDense` layer? It seems to be doing the same thing, yet the latter is a layer that would be saved when saving the model. 

Thank you!

Hi @LysandreJik,

Thanks for the information. I had the same observation 

> @Rocketknight1
> 
> https://github.com/huggingface/transformers/blob/408b2d2bd08f667cf4154730cc323c4e49657eed/src/transformers/modeling_tf_utils.py#L497-L514
> 
> Only the saving model layers are getting restored. ![Screenshot 2021-10-13 at 7 06 50 PM](https://user-images.githubusercontent.com/17096858/137143492-ac4a1797-aecf-404c-a51b-4bac33c5f001.png)
> 
> Other parameters are not listed in the `tf_model.h5` file also

I can't use [tf.EinsumDense](https://www.tensorflow.org/api_docs/python/tf/keras/layers/experimental/EinsumDense), because then it won't be an equivalent implementation to the PyTorch version and hence the PT_TF cross-test will fail. 

![Screenshot 2021-11-13 at 9 46 15 PM](https://user-images.githubusercontent.com/17096858/141651014-b9571594-3ca5-43ed-b5bc-fbab2951f20d.png)



Hi @kamalkraj, sorry for the delay. I did a full investigation here and here's what I found:

- The weights that are attached to the base `Model` with `self.add_weight()` are the only weights that are getting loaded incorrectly. All weights inside layers are loaded correctly.

- Even though attaching weights to the base model works in Keras in general, it doesn't work for our models because of how we build them. I think some part of the build process overwrites the loaded weights with randomly initialized ones.

- All of our other models resolve this issue by moving the weights from the main model class to other layers, e.g. a `QuestionAnsweringHead` Layer, but this may create issues with cross-loading weights from PyTorch. Even so, this is probably the solution we need here. I'll investigate the cross-loading code and try to find a solution there. Are you okay with moving those weights to a separate Layer?

> * I'll investigate the cross-loading code and try to find a solution there. Are you okay with moving those weights to a separate Layer?

Yes @Rocketknight1 

Thanks


@kamalkraj I checked the weight crossloading function, and we should be able to move the weights to another layer and as long as we name it correctly weight porting should work.

Thanks, @Rocketknight1 
Fixed the issue. 

@NielsRogge 
Could you please help me to upload the model weights to the official hub?


@kamalkraj sure! 

Btw I just discovered a (tiny) bug in the forward pass of TAPAS (when `config.select_one_column` is set to `False`). Let me open a PR for it first, such that you can include the fix in this PR.

@kamalkraj I'm uploading all TAPAS TF checkpoints to the hub.

Can you resolve the conflict shown above? Also, can you confirm the `test_pt_tf_model_equivalence` tests pass? They don't seem to pass on CI.

@NielsRogge 
![Screenshot 2021-11-29 at 7 28 08 PM](https://user-images.githubusercontent.com/17096858/143881532-cf26de03-6af6-4905-a661-8a48dcee1320.png)



@NielsRogge 
Test in CI failed due to version conflict of `Tensorflow Probability` with `Tensorflow`, Should I pin the version ?

The TF version should not be pinned, but the TF probability version can be pinned.

@NielsRogge 
In the setup.py the Tensorflow version is specified >= 2.3

https://github.com/kamalkraj/transformers/blob/fbad9bb56e8f67dca2c29fb21a5a017c823c57b7/setup.py#L155-L156

But in the CI Tensorflow installed is 2.6.2, Any idea 
https://app.circleci.com/pipelines/github/huggingface/transformers/30600/workflows/a636c0a9-a0f0-4fbe-8124-276b7ec5d6c5/jobs/312697?invite=true#step-111-4303

Current TF latest version is 2.7

It seems that this is due to pip not upgrading itself correctly:

```
WARNING: You are using pip version 21.2.4; however, version 21.3.1 is available.
You should consider upgrading via the '/usr/local/bin/python -m pip install --upgrade pip' command.
```

Could you try to update the following in your PR: https://github.com/huggingface/transformers/blob/25156eb296ae88c7b810235a368c953b7a4b9af9/.circleci/config.yml#L82
to
```
/usr/local/bin/python -m pip install --upgrade pip
```
to check if it changes anything? Thanks, @kamalkraj 


Thanks for trying it out, it seems like it didn't work out. I'll try a few things and come back to you.

Found the error! TensorFlow 2.7 does not support Python 3.6 anymore (cc @sgugger, @Rocketknight1, @patrickvonplaten, @patil-suraj).

Could you update this line: https://github.com/huggingface/transformers/blob/master/.circleci/config.yml#L68

to have `circleci/python:3.7` as an image?

Thanks @LysandreJik 

Should I revert this commit f18cfa9?

I have changed the python version only here ` run_tests_torch_and_tf `.  



@NielsRogge 
Thank you so much for uploading all the TF models to the hub. 
I have updated the tests to load the model directly from the hub, rather than using `from_pt`

Ready to merge 🤗 


Indeed, if you can revert the pip commit then we're ready to go! We can also merge it and revert it afterwards, do you want to take care of that @NielsRogge?

Fantastic @kamalkraj, let's merge it once it's all green