add sdpa to ViT by lyaronskaya · Pull Request #29325 · huggingface/transformers

lyaronskaya · 2024-02-27T17:45:14Z

What does this PR do?

Adding support for SDPA to ViT. Fixes #28005

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

fxmarty

LGTM

Can you run (preferably on a GPU):

RUN_SLOW=1 pytest tests/models/vit -k "test_eager_matches_sdpa_inference" -s -vvvvv

and report the result here?

fxmarty · 2024-02-27T18:04:47Z

Can you install ruff==0.1.5 and run make style & make fix-copies as well?

HuggingFaceDocBuilderDev · 2024-02-27T18:26:01Z

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

lyaronskaya · 2024-02-28T17:54:02Z

@fxmarty Hi!

Can you run (preferably on a GPU):
RUN_SLOW=1 pytest tests/models/vit -k "test_eager_matches_sdpa_inference" -s -vvvvv
and report the result here?

CPU results

collected 223 items / 220 deselected / 3 selected                                                                                                                                                          

tests/models/vit/test_modeling_vit.py::ViTModelTest::test_eager_matches_sdpa_inference_0_float16 <- tests/test_modeling_common.py SKIPPED (float16 not supported on cpu)
tests/models/vit/test_modeling_vit.py::ViTModelTest::test_eager_matches_sdpa_inference_1_bfloat16 <- tests/test_modeling_common.py PASSED
tests/models/vit/test_modeling_vit.py::ViTModelTest::test_eager_matches_sdpa_inference_2_float32 <- tests/test_modeling_common.py PASSED`

Can you install ruff==0.1.5 and run make style & make fix-copies as well?

Result of make fix-copies has inconsistencies, so I still have to check the fix-copies code.

ArthurZucker

Thanks! One way is either to remove the copied from and use another model as the base, or add sdpa to all of them.
I am down to add it to all of them and have @NielsRogge check once it's done!

ArthurZucker · 2024-02-29T10:45:35Z

src/transformers/models/audio_spectrogram_transformer/modeling_audio_spectrogram_transformer.py

        self.chunk_size_feed_forward = config.chunk_size_feed_forward
        self.seq_len_dim = 1
-        self.attention = ASTAttention(config)
+        self.attention = VIT_ATTENTION_CLASSES[config._attn_implementation](config)


fix copies is fixing this but VIT_ATTENTION_CLASSES does not exist

ArthurZucker · 2024-02-29T10:45:44Z

src/transformers/models/deit/modeling_deit.py

        self.chunk_size_feed_forward = config.chunk_size_feed_forward
        self.seq_len_dim = 1
-        self.attention = DeiTAttention(config)
+        self.attention = VIT_ATTENTION_CLASSES[config._attn_implementation](config)


ArthurZucker · 2024-02-29T10:45:49Z

src/transformers/models/videomae/modeling_videomae.py

        self.chunk_size_feed_forward = config.chunk_size_feed_forward
        self.seq_len_dim = 1
-        self.attention = VideoMAEAttention(config)
+        self.attention = VIT_ATTENTION_CLASSES[config._attn_implementation](config)


ArthurZucker · 2024-02-29T10:46:04Z

src/transformers/models/yolos/modeling_yolos.py

        self.chunk_size_feed_forward = config.chunk_size_feed_forward
        self.seq_len_dim = 1
-        self.attention = YolosAttention(config)
+        self.attention = VIT_ATTENTION_CLASSES[config._attn_implementation](config)


ArthurZucker · 2024-02-29T10:46:08Z

src/transformers/models/vit_msn/modeling_vit_msn.py

        self.chunk_size_feed_forward = config.chunk_size_feed_forward
        self.seq_len_dim = 1
-        self.attention = ViTMSNAttention(config)
+        self.attention = VIT_ATTENTION_CLASSES[config._attn_implementation](config)


lyaronskaya · 2024-03-04T17:01:31Z

@ArthurZucker @fxmarty Just added sdpa to all of these models that use ViT as base.
All tests passed except for one

FAILED tests/models/videomae/test_modeling_videomae.py::VideoMAEModelTest::test_eager_matches_sdpa_inference_1_bfloat16 - RuntimeError: "mse_cpu" not implemented for 'BFloat16'

ArthurZucker

A few changes are still to be removed!

src/transformers/models/audio_spectrogram_transformer/modeling_audio_spectrogram_transformer.py

src/transformers/models/deit/modeling_deit.py

src/transformers/models/videomae/modeling_videomae.py

src/transformers/models/vit_mae/modeling_vit_mae.py

src/transformers/models/vit/modeling_vit.py

src/transformers/models/vit_msn/modeling_vit_msn.py

src/transformers/models/yolos/modeling_yolos.py

src/transformers/models/audio_spectrogram_transformer/modeling_audio_spectrogram_transformer.py

ArthurZucker

You are right, sorry for the late answer!
Since you added _supports_sdpa slow test should be run!
Could you try RUN_SLOW=1 pytest tests/models/ with the changed models? 🤗

ArthurZucker · 2024-03-30T15:41:42Z

Can you also rebase on main to make sure CI is full green!

hyenal · 2024-04-27T16:32:13Z

Is there any update on this PR ? I d be happy to help with the remaining tasks (rebase + running the tests) if needs be :)

amyeroberts · 2024-04-29T10:21:13Z

Hi @hyenal, as this PR hasn't had any activity for over a month, feel free to open another PR with these changes and ping us for review when ready!

hyenal · 2024-04-29T11:45:24Z

Hi @hyenal, as this PR hasn't had any activity for over a month, feel free to open another PR with these changes and ping us for review when ready!

Thanks a lot for the reply, I d like to give the author a chance to reply to my message before submitting a new PR (even adding me as a contributor to the repo would work @lyaronskaya ).
Otherwise I will submit the PR by the end of the week :)

lyaronskaya · 2024-04-29T15:36:25Z

Hi @hyenal! I’m little busy, and you can take it over. I appreciate your help

remove blank line (+1 squashed commit) Squashed commits: [24ccd2061] [run-slow]vit_msn,vision_encoder_decoder (+24 squashed commits) Squashed commits: [08bd27e7a] [run-slow]vit_msn,vision_encoder_decoder [ec96a8db3] [run-slow]vit_msn [ead817eca] fix vit msn multi gpu [d12cdc8fd] [run-slow]audio_spectrogram_transformer,deit,vision_encoder_decoder,vision_text_dual_encoder,vit,vit_hybrid,vit_mae,vit_msn,videomae,yolos [3fdbfa88f] doc [a3ff33e4a] finish implementation [e20b7b7fb] Update test_modeling_common.py [e290c5810] Update test_modeling_flax_common.py [d3af86f46] comment [ff7dd32d8] more comments [59b137889] suggestion [7e2ba6d67] attn_implementation as attribute of the class [fe66ab71f] minor [38642b568] Apply suggestions from code review Accept comments Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> [22cde7d52] Update tests/test_modeling_common.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> [48e137cc6] Update tests/test_modeling_common.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> [99f4c679f] Update tests/test_modeling_common.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> [96cf20a6d] Update src/transformers/models/vit_msn/modeling_vit_msn.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> [c59377d23] Update src/transformers/models/vit_mae/modeling_vit_mae.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> [b70a47259] Update tests/models/vision_text_dual_encoder/test_modeling_vision_text_dual_encoder.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> [00c84d216] [run-slow]audio_spectrogram_transformer,deit,vision_encoder_decoder,vision_text_dual_encoder,vit,vit_hybrid,vit_mae,vit_msn,videomae,yolos [61f00ebb0] all tests are passing locally [e9e0b82b7] vision encoder/decoder [4d5076b56] test-vision (+20 squashed commits) Squashed commits: [d1add8db9] yolo [9fde65716] fix flax [986566c28] minor [ca2f21d1f] vit [3333efd7a] easy models change [ebfc21402] [run-slow]audio_spectrogram_transformer,deit,vision_encoder_decoder,vision_text_dual_encoder,vit,vit_hybrid,vit_mae,vit_msn,videomae,yolos [b8b8603ed] [run-slow]vision_encoder_decoder,vision_text_dual_encoder,yolos [48ecc7e26] all tests are passing locally [bff7fc366] minor [62f88306f] fix yolo and text_encoder tests [121507555] [run-slow]audio_spectrogram_transformer,deit,vit,vit_hybrid,vit_mae,vit_msn,videomae [1064cae0a] [run-slow]vision_encoder_decoder,vision_text_dual_encoder,yolos [b7f52ff3a] [run-slow]audio_spectrogram_transformer,deit,vit,vit_hybrid,vit_mae,vit_msn,videomae [cffaa10dd] fix-copies [ef6c511c4] test vit hybrid [7d4ba8644] vit hybrid [66f919033] [run-slow]audio_spectrogram_transformer,deit,vit,vit_hybrid,vit_mae,vit_msn,videomae [1fcc0a031] fixes [cfde6eb21] fixup [e77df1ed3] all except yolo end encoder decoder (+17 squashed commits) Squashed commits: [602913e22] vit + vit_mae are working [547f6c4cc] RUN_SLOW=1 pytest tests/models/audio_spectrogram_transformer/ tests/models/deit/ tests/models/videomae/ passes [61a97dfa9] it s the complete opposite... [aefab37d4] fix more tests [71802a1b9] fix all torch tests [40b12eb58] encoder - decoder tests [941552b69] slow decorator where appropriate [14d055d80] has_attentions to yolo and msn [3381fa19f] add correct name [e261316a7] repo consistency [31c6d0c08] fixup [9d214276c] minor fix [11ed2e1b7] chore [eca6644c4] add sdpa to vit-based models [cffbf390b] make fix-copies result [6468319b0] fix style [d324cd02a] add sdpa for vit Co-authored-by: Liubov Yaronskaya <luba.yaronskaya@gmail.com>

github-actions · 2024-05-24T08:05:00Z

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.

amyeroberts · 2024-05-24T10:42:24Z

Closing as superseded by #30555

add sdpa for vit

8c6846f

fxmarty approved these changes Feb 27, 2024

View reviewed changes

lyaronskaya added 2 commits February 28, 2024 18:34

fix style

d0069a0

make fix-copies result

cc529b9

ArthurZucker reviewed Feb 29, 2024

View reviewed changes

add sdpa to vit-based models

61f745b

chore

e84a4b8

ArthurZucker reviewed Mar 5, 2024

View reviewed changes

ArthurZucker reviewed Mar 27, 2024

View reviewed changes

src/transformers/models/audio_spectrogram_transformer/modeling_audio_spectrogram_transformer.py Show resolved Hide resolved

src/transformers/models/audio_spectrogram_transformer/modeling_audio_spectrogram_transformer.py Show resolved Hide resolved

ArthurZucker approved these changes Mar 30, 2024

View reviewed changes

huggingface deleted a comment from github-actions bot Apr 29, 2024

hyenal mentioned this pull request Apr 29, 2024

add sdpa to ViT [follow up of #29325] #30555

Merged

5 tasks

amyeroberts closed this May 24, 2024

Conversation

lyaronskaya commented Feb 27, 2024

What does this PR do?

Before submitting

Who can review?

Uh oh!

fxmarty left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fxmarty commented Feb 27, 2024

Uh oh!

HuggingFaceDocBuilderDev commented Feb 27, 2024

Uh oh!

lyaronskaya commented Feb 28, 2024

Uh oh!

ArthurZucker left a comment

Choose a reason for hiding this comment

Uh oh!

ArthurZucker Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

ArthurZucker Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

ArthurZucker Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

ArthurZucker Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

ArthurZucker Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

lyaronskaya commented Mar 4, 2024

Uh oh!

ArthurZucker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ArthurZucker left a comment

Choose a reason for hiding this comment

Uh oh!

ArthurZucker commented Mar 30, 2024

Uh oh!

hyenal commented Apr 27, 2024

Uh oh!

amyeroberts commented Apr 29, 2024

Uh oh!

hyenal commented Apr 29, 2024

Uh oh!

lyaronskaya commented Apr 29, 2024

Uh oh!

github-actions bot commented May 24, 2024

Uh oh!

amyeroberts commented May 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fxmarty left a comment •

edited

Loading