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.

Sign up for GitHub

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

Jump to bottom

[Bugfix] Fix dtype mismatch in PaliGemma #6367

Merged
merged 6 commits into from
Jul 12, 2024
Merged

[Bugfix] Fix dtype mismatch in PaliGemma #6367

merged 6 commits into from
Jul 12, 2024

Conversation

DarkLight1337
Copy link
Member

FIX #6366

@DarkLight1337 DarkLight1337 requested a review from ywang96 July 12, 2024 07:04
DarkLight1337
DarkLight1337 commented Jul 12, 2024
View reviewed changes
vllm/model_executor/models/paligemma.py
@@ -111,7 +111,7 @@ def input_processor_for_paligemma(ctx: InputContext, llm_inputs: LLMInputs):
orig_prompt = llm_inputs.get("prompt")
orig_prompt_ids = llm_inputs.get("prompt_token_ids")

if image_token_str in orig_prompt:
if orig_prompt is not None and image_token_str in orig_prompt:
Copy link
Member Author

Choose a reason for hiding this comment

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

My IDE flagged this so I fixed it along the way.

@ywang96
Copy link
Member

ywang96 commented Jul 12, 2024

Hmm that's weird - I thought the vision encoder and language model itself is always float32?

https://huggingface.co/google/paligemma-3b-mix-224/blob/main/config.json

@DarkLight1337
Copy link
Member Author

DarkLight1337 commented Jul 12, 2024 •
edited
Loading

Hmm that's weird - I thought the vision encoder and language model itself is always float32?

https://huggingface.co/google/paligemma-3b-mix-224/blob/main/config.json

Doesn't vLLM update the dtype regardless of how it's defined on HF? (Didn't check the code but since we have an option to set the dtype via LLM args, that should be the case right?)

@ywang96
Copy link
Member

ywang96 commented Jul 12, 2024

I wonder why we never needed to do this for Llava models - let me test the original issue and this PR and see if this fixes it.

I do think we should test this model on float32 though.

@ywang96 ywang96 merged commit 024ad87 into vllm-project:main Jul 12, 2024
72 checks passed
@DarkLight1337 DarkLight1337 deleted the fix-paligemma branch July 13, 2024 02:16
dtrifiro pushed a commit to opendatahub-io/vllm that referenced this pull request Jul 17, 2024
@DarkLight1337 @dtrifiro
[Bugfix] Fix dtype mismatch in PaliGemma (vllm-project#6367)
a68d100
xjpang pushed a commit to xjpang/vllm that referenced this pull request Jul 24, 2024
@DarkLight1337
[Bugfix] Fix dtype mismatch in PaliGemma (vllm-project#6367)
4257eb3
Alvant pushed a commit to compressa-ai/vllm that referenced this pull request Oct 26, 2024
@DarkLight1337 @Alvant
[Bugfix] Fix dtype mismatch in PaliGemma (vllm-project#6367)
be33c13 
Signed-off-by: Alvant <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ywang96 ywang96 ywang96 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@DarkLight1337 @ywang96