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

Add TAPEX #16473

Merged
merged 33 commits into from Apr 8, 2022
Merged

Add TAPEX #16473

merged 33 commits into from Apr 8, 2022

Conversation

NielsRogge
Copy link
Contributor

@NielsRogge NielsRogge commented Mar 29, 2022

What does this PR do?

Remember TAPAS, the table QA model by Google AI? Microsoft has now released TAPEX, a seq2seq model that outperforms TAPAS and is actually much simpler: table QA is just treated as a seq2seq problem.

As the weights can be directly loaded into a BART model, this PR only implements TapexTokenizer, which can be used to prepare tables and corresponding texts for the model.

This PR also adds 3 scripts that showcase how to fine-tune TAPEX on 3 important benchmarks: WikiSQL and WTQ for table question answering and TabFact for table fact verification.

Kudos to @SivilTaram (the original author) for improving my initial TapexTokenizer implementation, as well as adding the 3 fine-tuning scripts.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Mar 29, 2022

The documentation is not available anymore as the PR was closed or merged.

Copy link
Contributor

@patil-suraj patil-suraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for adding this super cool model!
LGTM, just left a few nits.

docs/source/model_doc/tapex.mdx Outdated Show resolved Hide resolved
src/transformers/models/tapex/__init__.py Outdated Show resolved Hide resolved
src/transformers/models/tapex/tokenization_tapex.py Outdated Show resolved Hide resolved
src/transformers/models/tapex/tokenization_tapex.py Outdated Show resolved Hide resolved
tests/tapex/test_tokenization_tapex.py Outdated Show resolved Hide resolved
examples/research_projects/tapex/run_wikisql_with_tapex.py Outdated Show resolved Hide resolved
examples/research_projects/tapex/run_wikisql_with_tapex.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

If the expected architecture should be BART by default, then this model should be added in the relevant auto mapping to work with AutoConfig and AutoModelForXxx. This is jsut a defult value that can be changed in the config if there are checkpoints that rely on a different architecture.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/source/model_doc/tapex.mdx Outdated Show resolved Hide resolved
docs/source/model_doc/tapex.mdx Outdated Show resolved Hide resolved
docs/source/model_doc/tapex.mdx Outdated Show resolved Hide resolved
tests/tapex/test_tokenization_tapex.py Outdated Show resolved Hide resolved
tests/tapex/test_tokenization_tapex.py Outdated Show resolved Hide resolved
tests/tapex/test_tokenization_tapex.py Outdated Show resolved Hide resolved
tests/tapex/test_tokenization_tapex.py Outdated Show resolved Hide resolved
tests/tapex/test_tokenization_tapex.py Outdated Show resolved Hide resolved
@NielsRogge
Copy link
Contributor Author

NielsRogge commented Mar 29, 2022

make fixup is complaining:

examples/research_projects/tapex/run_wikitablequestions_with_tapex.py:50:1: F401 'wikisql_utils._TYPE_CONVERTER' imported but unused
examples/research_projects/tapex/run_wikitablequestions_with_tapex.py:50:1: F401 'wikisql_utils.retrieve_wikisql_query_answer_tapas' imported but unused

However, these functions are used in the script, so I can't remove these imports.

@SivilTaram
Copy link
Contributor

@NielsRogge Thanks for your huge effort! I personally think these two warnings are correct since these two imports are only used in run_wikisql_with_tapex.py instead of run_wikitablequestions_with_tapex.py (the hint message). I think we can remove them.

examples/research_projects/tapex/run_wikitablequestions_with_tapex.py:50:1: F401 'wikisql_utils._TYPE_CONVERTER' imported but unused
examples/research_projects/tapex/run_wikitablequestions_with_tapex.py:50:1: F401 'wikisql_utils.retrieve_wikisql_query_answer_tapas' imported but unused

@NielsRogge
Copy link
Contributor Author

@sgugger I've addressed all comments.

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model is still missing in the auto configuration and auto model API to provide the Bart default, which means that it won't work with AutoModelForSeq2Seq for instance. This should be added before merging.

examples/research_projects/tapex/README.md Show resolved Hide resolved
@NielsRogge
Copy link
Contributor Author

NielsRogge commented Apr 4, 2022

All models on the hub do work with the Auto API, can you elaborate? TAPEX is also added to configuration_auto.py and modeling_auto.py.

@NielsRogge NielsRogge force-pushed the add_tapex_bis branch 2 times, most recently from b078744 to 5f86fa5 Compare April 7, 2022 08:36
Niels Rogge and others added 18 commits April 8, 2022 08:33
…estion answering and table-based fact verification.
…kground.

- Remove unused code lines in tabfact script.
- Disable the deafult `pad_to_max_length` option which is memory-consuming.
* Fix the do_lower_case behaviour of TapexTokenizer.
* Add unit tests for target scenarios and cased/uncased scenarios for both source and target.
…enizer function.

* Fix typos in tapex example README.
…zer to control whether do_lower_case

* Guarantee the hyper-parameter can be run without out-of-memory on 16GB card and report the new reproduced number on wikisql
* Provide evaluation command.
@NielsRogge NielsRogge merged commit 4ef0abb into huggingface:main Apr 8, 2022
elusenji pushed a commit to elusenji/transformers that referenced this pull request Apr 8, 2022
* Add TapexTokenizer

* Improve docstrings and provide option to provide answer

* Remove option for pretokenized inputs

* Add TAPEX to README

* Fix copies

* Remove option for pretokenized inputs

* Initial commit: add tapex fine-tuning examples on both table-based question answering and table-based fact verification.

* - Draft a README file for running the script and introducing some background.
- Remove unused code lines in tabfact script.
- Disable the deafult `pad_to_max_length` option which is memory-consuming.

* * Support `as_target_tokenizer` function for TapexTokenizer.
* Fix the do_lower_case behaviour of TapexTokenizer.
* Add unit tests for target scenarios and cased/uncased scenarios for both source and target.

* * Replace the label BartTokenizer with TapexTokenizer's as_target_tokenizer function.
* Fix typos in tapex example README.

* * fix the evaluation script - remove the property `task_name`

* * Make the label space more clear for tabfact tasks

* * Using a new fine-tuning script for tapex-base on tabfact.

* * Remove the lowercase code outside the tokenizer - we use the tokenizer to control whether do_lower_case
* Guarantee the hyper-parameter can be run without out-of-memory on 16GB card and report the new reproduced number on wikisql

* * Remove the default tokenizer_name option.
* Provide evaluation command.

* * Support for WikiTableQuestion dataset.

* Fix a typo in README.

* * Fix the datasets's key name in WikiTableQuestions

* Run make fixup and move test to folder

* Fix quality

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Suraj Patil <surajp815@gmail.com>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

* Apply some more suggestions from code review

* Improve docstrings

* Overwrite failing test

* Improve comment in example scripts

* Fix rebase

* Add TAPEX to Auto mapping

* Add TAPEX to auto config mappings

* Put TAPEX higher than BART in auto mapping

* Add TAPEX to doc tests

Co-authored-by: Niels Rogge <nielsrogge@Nielss-MBP.localdomain>
Co-authored-by: SivilTaram <qianlxc@outlook.com>
Co-authored-by: Niels Rogge <nielsrogge@nielss-mbp.home>
Co-authored-by: Suraj Patil <surajp815@gmail.com>
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Co-authored-by: Niels Rogge <nielsrogge@Nielss-MacBook-Pro.local>
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

5 participants