Repository navigation
perf(qwen_vl): multimodal RoPE leaks f32 into the decode residual #1714
Description
Problem
apply_multimodal_rotary_pos_emb in src/models/qwen2_vl.rs:225, src/models/qwen3_vl.rs:226 and src/models/qwen3_vl_moe.rs:244 multiplied f16/bf16 queries and keys by f32 cos/sin tables and returned the promoted result. The tables are f32 on purpose (widened inv_freq and position ids), so the arithmetic is correct; the defect is that the promoted value escaped the helper into the residual stream and forced every later matmul to promote its own half-precision weight.
Fourth instance of the class fixed in #1709 (bridge activation helpers), #1710 (Phi/StableLM attention guard) and #1711 (attention scale promoting the query). The invariant is that a helper returns the dtype it was given: promotion inside a compiled graph is free, only promotion that escapes a function boundary costs bandwidth. Upstream mlx-vlm computes in float32 and then casts back under a cast_output flag (q_embed = q_embed.astype(q.dtype)) in https://github.com/Blaizzy/mlx-vlm/blob/main/mlx_vlm/models/rope_utils.py; the port took the arithmetic and dropped the restore.
Only the image path is affected: the text-only fast path (can_use_text_only_fast_path / forward_text_only_hidden) uses ordinary 1-D rope and never enters this function, so text throughput looked healthy while image decode was slow. Once an image sets position_ids the MRoPE path is sticky for the rest of the sequence.
Fix
Cast q_embed and k_embed back to array_dtype(q) / array_dtype(k) before returning, in all three files. Committed as 2f4fbab on bench/0.7.0-refresh (PR #1617).
Measurement
M1 Ultra, decode tok/s with an image versus the same prompt without one:
| Checkpoint | Before | After | Ratio |
|---|---|---|---|
| qwen2.5-vl-3b-hf | 19.67 | 72.21 | 0.25x to 0.93x |
| qwen2.5-vl-3b-4bit | 123.62 | 145.30 | 0.74x to 0.87x |
| qwen2-vl-2b-4bit | 183.34 | 217.73 | 0.72x to 0.85x |
| qwen3-vl-4b-instruct-4bit | 112.91 | 116.90 | 0.87x to 0.90x |
The bf16 checkpoint gains most: it is the only non-quantized one in the family, so the promotion reached the full weight set rather than just the scales.
Verification
Three-colour fixtures (blue, green, purple) against mlx-vlm as the reference. qwen2.5-vl-3b-hf answers Blue/Green/Purple, matching mlx-vlm exactly. qwen2-vl-2b-4bit answers Blue/Blue/Blue, exactly what mlx-vlm answers for that checkpoint; the pre-fix v0.6.0 binary answered Blue/White/Blue, further from the reference. The test is agreement with the reference implementation, not whether the answer is right.
Do not generalize this
src/vision/encoders/gemma3n.rs:58 has a similar-looking f32 promotion that must NOT be removed. It is load-bearing: mlxcel converts that tower's bf16 weights to f16 and its residuals reach about 213055, past the f16 max of 65504. A static audit cannot separate the two cases; only reading the surrounding code or measuring can.
Activity
perf(vlm): restore the input dtype after the 3D rotary
inureyes commentedon Sep 8, 2026
Confirmed on a second machine, and the sibling defect in the same class is fixed alongside it.
M5 Max, decode tok/s on the image path, measured by the M5 session against the same reference stack: paddleocr-vl-bfloat16 157.29 to 455.19 (2.89x), glm-4.5v-4bit 17.67 to 57.65 (3.26x), glm-4.1v-9b-thinking-4bit 69.16 to 86.80 (1.26x), qwen2.5-vl-3b-4bit 171.09 to 205.59 (1.20x), qwen2-vl-2b-4bit 290.55 to 336.67 (1.16x), qwen3-vl-4b-instruct-4bit 148.73 to 177.42 (1.19x). Text moves by the same margins.
The ordering is the same on both hosts and it is the ordering the mechanism predicts: non-quantized gains most, then 4-bit MoE, then 4-bit dense, in proportion to how much weight the promotion reaches. On M1 Ultra the same three files gave paddleocr-vl-bfloat16 135.97 to 333.35 and glm-4.5v-4bit 18.24 to 34.73. There is no machine-specific term here.
Two corrections to the record, both mine to carry.
An earlier round of M5 numbers showed the fix as neutral, at 1.00x across the board. That result was invalid: the post-fix arm reused a sweep taken before the build was rebased, so both arms ran the same pre-fix binary and their agreement was guaranteed rather than measured. The benchmark harness stamps
mlxcel_commitfrom git, not from the binary, so a row can name a commit the measured binary does not contain. The check that catches it is a no-opcargo buildimmediately before measuring; the gemma3n rows are the control that localises the damage, because that change was compiled into the old binary and those rows did not move.The bandwidth reading built on those numbers is withdrawn with them. The corrected M5 profile has
async_evalfalling from 6.175 to 1.976 ms/tok, so the device time is where the gain lands, the same as on M1 Ultra.Fixed in
2f4fbabbc(Qwen-VL) and5287eb9a2(paddleocr_vl, glm4v, falcon_ocr_rope) onbench/0.7.0-refresh.inureyes commentedon Oct 15, 2026
Closing as fixed on main. The cast back to the input dtype landed in PR #1617 (merged 2026-09-08, commit 2f4fbab is an ancestor of
main), but that PR had no closing keyword for this issue, so it stayed open instatus:review.Checked on
mainat fe98588:apply_multimodal_rotary_pos_embinsrc/models/qwen2_vl.rs,src/models/qwen3_vl.rsandsrc/models/qwen3_vl_moe.rsall end withastype(&q_embed, array_dtype(q))/astype(&k_embed, array_dtype(k)). Commit 5287eb9 later applied the same fix to glm4v, paddleocr_vl and falcon_ocr rope. The measurements and the mlx-vlm agreement check are in the issue body and PR #1617.