transformers icon indicating copy to clipboard operation
transformers copied to clipboard

Add ViViT

Open jegork opened this issue 2 years ago • 10 comments

What does this PR do?

Fixes #15666. Reopening #20441, as I have missed the comments provided by @amyeroberts so the issue was closed by the bot.

Add Video Vision Transformer to transformers. This PR implements a spacetime version of the Video Vision Transformer from the original paper.

I have provided the model weights here https://huggingface.co/jegormeister/vivit-b-16x2-kinetics400 I will try to add Factorised Encoder version later on (these are the two versions that authors provide weight for).

Before submitting

  • [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • [x] Did you read the contributor guideline, Pull Request section?
  • [x] Was this discussed/approved via a Github issue or the forum? https://github.com/huggingface/transformers/issues/15666
  • [x] Did you make sure to update the documentation with your changes? I have added the documentation, but I have troubles testing it as I couldn't run the preview command of the doc-builder, so if someone has the possibility to run and check it, I will be really grateful!
  • [x] Did you write any new necessary tests? WIP

Who can review?

@amyeroberts provided the last suggestions to the closed PR, so I hope you can review this one. Thanks!

jegork avatar Apr 02 '23 15:04 jegork

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

@amyeroberts thank you for your comments in the previous PR! I have addressed your suggestions and also added the image processor test.

However, I ran make style, however the pipeline was failing at check_code_quality. Therefore I've updated the testing dependencies which lead to black updating. But! When I run make style again, it gives the following output:

reformatted transformers/examples/research_projects/deebert/src/modeling_highway_bert.py
reformatted transformers/examples/research_projects/movement-pruning/emmental/modeling_bert_masked.py
reformatted transformers/src/transformers/models/reformer/modeling_reformer.py
reformatted transformers/src/transformers/models/vivit/modeling_vivit.py
reformatted transformers/tests/models/vivit/test_image_processing_vivit.py

So it fixes not only the files from this PR, but also the already existing ones. Therefore I have a question: should I only pus reformatted files from this PR or all?

jegork avatar Apr 02 '23 15:04 jegork

@jegork The files listed e.g. transformers/examples/research_projects/deebert/src/modeling_highway_bert.py should have the most recent formatting applied and shouldn't need to be updated with this PR.

Could you rebase on main, make sure the most recent formatting packages are installed using pip install -e .[quality] and try make style again?

amyeroberts avatar Apr 03 '23 12:04 amyeroberts

@amyeroberts Thanks for your comments. I have addressed everything. However, I still get the same problems with make style I did git fetch upstream, then git rebase upstream/main, pip install -e ".[quality]" after which I ran make style

Which resulted in the following output at the end:

black examples tests src utils setup.py
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running ``pip install "black[jupyter]"``
reformatted /Users/jegorkitskerkin/Documents/projects/transformers/examples/research_projects/deebert/src/modeling_highway_bert.py
reformatted /Users/jegorkitskerkin/Documents/projects/transformers/examples/research_projects/movement-pruning/emmental/modeling_bert_masked.py
reformatted /Users/jegorkitskerkin/Documents/projects/transformers/src/transformers/models/vivit/modeling_vivit.py
reformatted /Users/jegorkitskerkin/Documents/projects/transformers/src/transformers/models/reformer/modeling_reformer.py
reformatted /Users/jegorkitskerkin/Documents/projects/transformers/tests/models/vivit/test_image_processing_vivit.py

All done! ✨ 🍰 ✨
5 files reformatted, 2380 files left unchanged.
ruff examples tests src utils setup.py --fix
/Library/Developer/CommandLineTools/usr/bin/make autogenerate_code
running deps_table_update
updating src/transformers/dependency_versions_table.py
/Library/Developer/CommandLineTools/usr/bin/make extra_style_checks
python utils/custom_init_isort.py
python utils/sort_auto_mappings.py
doc-builder style src/transformers docs/source --max_len 119 --path_to_docs docs/source
Overwriting content of src/transformers/models/vivit/modeling_vivit.py.
Cleaned 1 files!
python utils/check_doc_toc.py --fix_and_overwrite

As you can see, the same unrelated-to-this-PR-files are getting formatted

jegork avatar May 01 '23 23:05 jegork

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 are likely to be ignored.

github-actions[bot] avatar Jun 12 '23 15:06 github-actions[bot]

hey @amyeroberts i've managed to fix the formatting-related problems and addressed your comments. However, I seem to be facing some problems with running tests. CI fails with

FAILED tests/models/vivit/test_modeling_vivit.py::VivitModelTest::test_model_outputs_equivalence - Failed: Timeout >120.0s

and as I see, test_model_outputs_equivalence comes from the ModelTesterMixin so I am not sure how to handle this

jegork avatar Jun 19 '23 12:06 jegork

@jegork Mmmm, indeed that's odd. It's not immediately clear from the CI traceback why that would happen.

Are you able to run the tests locally and do they pass?:

RUN_SLOW=1 tests/models/vivit/test_modeling_vivit.py::VivitModelTest::test_model_outputs_equivalence

amyeroberts avatar Jun 19 '23 17:06 amyeroberts

@amyeroberts yep, everything works and passes locally

jegork avatar Jun 19 '23 17:06 jegork

@jegork Thanks for confirming. I'm going to rerun CircleCI in case there was just some transient issue with the run. If it persists we can dig a bit more into it.

amyeroberts avatar Jun 19 '23 17:06 amyeroberts

Thanks @amyeroberts and @jegork for working on this, we look forward to using the ViVit model!

adildahlan avatar Jun 24 '23 02:06 adildahlan

@jegork Is the PR OK to merge? Or are there any other commits you'd like to push before I press the big green button? 🟢

amyeroberts avatar Jul 11 '23 12:07 amyeroberts

@amyeroberts I think it's ready to be merged. Thanks for your help!

jegork avatar Jul 11 '23 12:07 jegork

Hi @jegork congrats on your amazing contribution!

is it ok if we transfer the ViViT checkpoints to the google organization on the hub? (assuming they are officially released checkpoints by Google)

NielsRogge avatar Jul 18 '23 16:07 NielsRogge

Hey @NielsRogge, thanks!

Sure

jegork avatar Jul 18 '23 20:07 jegork