Skip to content

Small fix for Qwen3-VL to run vision sft. - #4705

Open
lkhphuc wants to merge 1 commit into
AI-Hypercomputer:mainfrom
lkhphuc:qwen3-mm-fix
Open

Small fix for Qwen3-VL to run vision sft.#4705
lkhphuc wants to merge 1 commit into
AI-Hypercomputer:mainfrom
lkhphuc:qwen3-mm-fix

Conversation

@lkhphuc

@lkhphuc lkhphuc commented Aug 3, 2026

Copy link
Copy Markdown

Description

Minor fix to get Qwen3-VL-2B running with vision sft.

Currently if run python -m maxtext.trainers.post_train.sft.train_sft_native src/maxtext/configs/post_train/sft-vision-chartqa.yml model_name=qwen3-vl-2b we get error:

  File "/lustre1/tier2/users/phuc.lekhac/maxtext/src/maxtext/trainers/pre_train/train.py", line 189, in loss_fn
    logits = model(
             ^^^^^^
  File "/lustre1/tier2/users/phuc.lekhac/maxtext/src/maxtext/models/models.py", line 552, in __call__
    logits, hidden_state, kv_caches = self.decoder(
                                      ^^^^^^^^^^^^^
  File "/lustre1/tier2/users/phuc.lekhac/maxtext/src/maxtext/layers/nnx_decoders.py", line 1540, in __call__
    y = self._apply_embedding(
        ^^^^^^^^^^^^^^^^^^^^^^
  File "/lustre1/tier2/users/phuc.lekhac/maxtext/src/maxtext/layers/nnx_decoders.py", line 1312, in _apply_embedding
    y = mm_utils.merge_mm_embeddings(
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lustre1/tier2/users/phuc.lekhac/maxtext/src/maxtext/multimodal/utils.py", line 172, in merge_mm_embeddings
    raise ValueError(
ValueError: token_masks must contain either one value per multimodal token or one value per tile. Got 3 mask values for 576 embeddings.

When lowering/compile, the image mask is always passed down:
shaped_batch["image_masks"] = ShapeDtypeStruct(image_shape[:2], ...)

Currently, this image_mask is only supposed to be used with llama4-tiling with input (B*N, tiles, C, H, W), so I gated it with the llama4 model name.
Gemma 4 return image_shape (B, N, H, W, C) and accidentally run, but semantic it's still wrong.
Qwen3 return image_shape (B, C, T, H, W) with C=3 thus fail with that error.

Tests

Added a small test to make sure that gate

Checklist

Before submitting this PR, please make sure (put X in square brackets):

  • I have performed a self-review of my code. For an optional AI review, add the gemini-review label.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run end-to-end tests tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed, including adding new documentation pages to the relevant Table of Contents (toctree directive) as explained in our documentation.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@hengtaoguo hengtaoguo 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.

Thanks for the bug fix! Do you have any end2end test runs to validate the change?

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@hengtaoguo

Copy link
Copy Markdown
Collaborator

Could you also sync to head before submission? The failing tests are unrelated to your PR and we've skipped them.

@aireenmei aireenmei 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.

Thanks for the fix, wonder if qwen3-VL SFT work after this fix?

@lkhphuc

lkhphuc commented Aug 4, 2026

Copy link
Copy Markdown
Author

Thanks. I rebased on main.
I am not aware of any integration test for this. I basically just run the chartqa sft with qwen3-vl like:
python -m maxtext.trainers.post_train.sft.train_sft_native src/maxtext/configs/post_train/sft-vision-chartqa.yml model_name=qwen3-vl-2b

With this fix, qwen3-vl-2b can run through a few steps, but you still need this PR #4709 for it to be correct.

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.

3 participants