Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tapas tf #13393

Merged
merged 39 commits into from
Nov 30, 2021
Merged

Tapas tf #13393

merged 39 commits into from
Nov 30, 2021

Conversation

kamalkraj
Copy link
Contributor

What does this PR do?

TF Tapas

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,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? 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, and
    here are tips on formatting docstrings.
  • 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.

@NielsRogge @sgugger @LysandreJik @Rocketknight1

@kamalkraj
Copy link
Contributor Author

Screenshot 2021-09-02 at 9 11 41 PM

@kamalkraj
Copy link
Contributor Author

kamalkraj commented Sep 2, 2021

pending

  • adding unit test cases
  • adding model to TFAutoTableQuestionAnswering pipeline
  • adding unit test for the pipeline
  • fix all the copy pasted code comments from pt->tf
  • update tapas.rst with TF sample code
  • add #Copied Comments`
  • push tf weights to official model hub - help needed.
  • update MLM model, Add TAPAS MLM-only models #13408
  • ...

@kamalkraj kamalkraj marked this pull request as ready for review September 4, 2021 13:55
@kamalkraj kamalkraj force-pushed the tapas-tf branch 2 times, most recently from 13269bf to 477463f Compare September 8, 2021 13:40
@kamalkraj
Copy link
Contributor Author

@LysandreJik
ready for review 🤗

@LysandreJik
Copy link
Member

Great news @kamalkraj!

@NielsRogge and @Rocketknight1, could you take a look at this?

@Rocketknight1
Copy link
Member

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.

@kamalkraj
Copy link
Contributor Author

kamalkraj commented Nov 22, 2021

Thanks, @Rocketknight1
Fixed the issue.

@NielsRogge
Could you please help me to upload the model weights to the official hub?

@NielsRogge
Copy link
Contributor

NielsRogge commented Nov 23, 2021

@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.

@NielsRogge NielsRogge mentioned this pull request Nov 23, 2021
@NielsRogge
Copy link
Contributor

NielsRogge commented Nov 29, 2021

@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.

@kamalkraj
Copy link
Contributor Author

@NielsRogge
Screenshot 2021-11-29 at 7 28 08 PM

@kamalkraj
Copy link
Contributor Author

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

@NielsRogge
Copy link
Contributor

NielsRogge commented Nov 29, 2021

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

@kamalkraj
Copy link
Contributor Author

kamalkraj commented Nov 29, 2021

@LysandreJik
Copy link
Member

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:

- run: pip install --upgrade pip

to

/usr/local/bin/python -m pip install --upgrade pip

to check if it changes anything? Thanks, @kamalkraj

@LysandreJik
Copy link
Member

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

@LysandreJik
Copy link
Member

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?

@kamalkraj
Copy link
Contributor Author

Thanks @LysandreJik

Should I revert this commit f18cfa9?

I have changed the python version only here run_tests_torch_and_tf.

@kamalkraj
Copy link
Contributor Author

@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 🤗

@LysandreJik
Copy link
Member

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?

@LysandreJik
Copy link
Member

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

@NielsRogge NielsRogge merged commit c468a87 into huggingface:master Nov 30, 2021
Albertobegue pushed a commit to Albertobegue/transformers that referenced this pull request Jan 27, 2022
* TF Tapas first commit

* updated docs

* updated logger message

* updated pytorch weight conversion
script to support scalar array

* added use_cache to tapas model config to
work properly with tf input_processing

* 1. rm embeddings_sum
2. added # Copied
3. + TFTapasMLMHead
4. and lot other small fixes

* updated docs

* + test for tapas

* updated testing_utils to check
is_tensorflow_probability_available

* converted model logits post processing using
numpy to work with both PT and TF models

* + TFAutoModelForTableQuestionAnswering

* added TF support

* added test for
TFAutoModelForTableQuestionAnswering

* added test for
TFAutoModelForTableQuestionAnswering pipeline

* updated auto model docs

* fixed typo in import

* added tensorflow_probability to run tests

* updated MLM head

* updated tapas.rst with TF  model docs

* fixed optimizer import in docs

* updated convert to np
data from pt model is not
`transformers.tokenization_utils_base.BatchEncoding`
after pipeline upgrade

* updated pipeline:
1. with torch.no_gard removed, pipeline forward handles
2. token_type_ids converted to numpy

* updated docs.

* removed `use_cache` from config

* removed floats_tensor

* updated code comment

* updated Copyright Year and
logits_aggregation Optional

* updated docs and comments

* updated docstring

* fixed model weight loading

* make fixup

* fix indentation

* added tf slow pipeline test

* pip upgrade

* upgrade python to 3.7

* removed from_pt from tests

* revert commit f18cfa9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants