Skip to content

[blip-2] Fix dtype mismatch when keep in fp32 #37068

Merged
zucchini-nlp merged 4 commits into
huggingface:mainfrom
zucchini-nlp:fix-blip2-keep-in-fp32
Mar 28, 2025
Merged

[blip-2] Fix dtype mismatch when keep in fp32 #37068
zucchini-nlp merged 4 commits into
huggingface:mainfrom
zucchini-nlp:fix-blip2-keep-in-fp32

Conversation

@zucchini-nlp

@zucchini-nlp zucchini-nlp commented Mar 28, 2025

Copy link
Copy Markdown
Member

What does this PR do?

Seems that BLIP2 keep_in_fp32_modules was not picking up correctly until 36722 (thus no errors up to today). Now that the query_tokens are actually kept in 32, we are getting dtype mismatch error in inference. (reported by @hmellor)

This PR fixes by casting inputs to correct dtypes, and keeping qformer in fp32 as well. The fix was tested with slow BLIP-2 tests and vLLM tests

@github-actions
github-actions Bot marked this pull request as draft March 28, 2025 09:43
@github-actions

Copy link
Copy Markdown
Contributor

Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. The CI will be paused while the PR is in draft mode. When it is ready for review, please click the Ready for review button (at the bottom of the PR page). This will assign reviewers and trigger CI.

@zucchini-nlp
zucchini-nlp requested a review from hmellor March 28, 2025 09:43
@zucchini-nlp
zucchini-nlp marked this pull request as ready for review March 28, 2025 09:44

@hmellor hmellor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

I have confirmed that the vLLM test now passes on the latest commit of this PR 🚀

@ArthurZucker ArthurZucker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's add a test please! 🤗

@zucchini-nlp

Copy link
Copy Markdown
Member Author

Test is already added, slow tests were failing on main from the above linked PR. The reason is that we don't usually run slow tests much 🥲

@ArthurZucker

Copy link
Copy Markdown
Collaborator

Let's merge then!

@zucchini-nlp
zucchini-nlp merged commit 52cc204 into huggingface:main Mar 28, 2025
@zucchini-nlp zucchini-nlp added the for patch Tag issues / labels that should be included in the next patch label Mar 28, 2025
ArthurZucker pushed a commit that referenced this pull request Mar 28, 2025
* fix fp32 BLIP2

* no need to reorder that

* check for `Noneness` as well before casting dtype
zucchini-nlp added a commit to zucchini-nlp/transformers that referenced this pull request May 14, 2025
* fix fp32 BLIP2

* no need to reorder that

* check for `Noneness` as well before casting dtype
soghomon-b pushed a commit to soghomon-b/transformers that referenced this pull request Aug 24, 2025
* fix fp32 BLIP2

* no need to reorder that

* check for `Noneness` as well before casting dtype
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

for patch Tag issues / labels that should be included in the next patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants