Skip to content

Fix Qwen3.5 VLM garbage output: gate the RMSNorm weight shift in sanitize#403

Merged
davidkoski merged 1 commit into
ml-explore:mainfrom
GoodOlClint:pr/qwen35-vlm-sanitize-normshift
Jul 13, 2026
Merged

Fix Qwen3.5 VLM garbage output: gate the RMSNorm weight shift in sanitize#403
davidkoski merged 1 commit into
ml-explore:mainfrom
GoodOlClint:pr/qwen35-vlm-sanitize-normshift

Conversation

@GoodOlClint

Copy link
Copy Markdown
Contributor

Proposed changes

MLXVLM/Models/Qwen35.sanitize applies the RMSNorm +1 weight offset unconditionally, whereas MLXLLM/Models/Qwen35.sanitize gates it behind hasMTPWeights || hasUnsanitizedConv1d.

On an already-converted MLX checkpoint (conv1d weights already sanitized to a trailing dim of 1, no MTP tensors), the norm weights have already been shifted at conversion time. The VLM path shifts them a second time, doubling every input_layernorm / post_attention_layernorm / q_norm / k_norm / model.norm. That decoheres the model and produces garbage tokens.

The same weights loaded through the MLXLLM Qwen35 path (which gates the shift) decode correctly — which isolates the divergence to sanitize, not the model math or the shared gated-delta kernel. Because Qwen3.5 is natively multimodal, every Qwen3.5 checkpoint routes through VLMModelFactory → the affected path, so all Qwen3.5 generation (text and multimodal) is broken on pre-converted checkpoints.

Fix

Mirror the MLXLLM gate in the VLM sanitize: compute shouldShiftNormWeights = hasMTPWeights || hasUnsanitizedConv1d from the incoming weights and apply the +1 only when true. This makes the VLM path reproduce the MLXLLM numerics.

Reproduction

  • Load a pre-converted Qwen3.5 VLM checkpoint via VLMModelFactory and greedily decode a trivial prompt → garbage before, coherent after.
  • The same weights with vision_config removed route to LLMModelFactory and were already coherent (the control).

Tests

Adds two Qwen35SanitizeTests cases: a pre-converted checkpoint (conv1d already sanitized) must not shift the norm weights; a raw checkpoint (unsanitized conv1d) must. Verified end-to-end on the previously-failing checkpoint (garbage → coherent).

xcodebuild test -scheme mlx-swift-lm-Package -destination 'platform=macOS' \
  -only-testing:MLXLMTests/Qwen35SanitizeTests

Checklist

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation (not needed)

🤖 Generated with Claude Code

MLXVLM Qwen35.sanitize applied the RMSNorm `+1` weight offset
unconditionally, while MLXLLM Qwen35 gates it behind
`hasMTPWeights || hasUnsanitizedConv1d`. On a pre-converted MLX
checkpoint (conv1d already sanitized, no MTP tensors) the weights are
already shifted, so the VLM path double-shifted every layernorm and
produced garbage tokens — while the same weights via the MLXLLM path
decoded correctly. Mirror the LLM gate so the two paths agree.

Adds sanitize unit tests covering both the pre-converted (no shift) and
raw-checkpoint (shift) cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@GoodOlClint GoodOlClint force-pushed the pr/qwen35-vlm-sanitize-normshift branch from b01c6b4 to 90b8b1b Compare July 9, 2026 18:14

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

Changes look good, thank you!

@davidkoski davidkoski merged commit 2f95038 into ml-explore:main Jul 13, 2026
2 checks passed
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.

2 participants