Skip to content

fix(vision): capture the functional slice_update return in DeepStack and MRoPE-section loops#666

Merged
inureyes merged 1 commit into
mainfrom
fix/issue-650-slice-update-injection
Jul 5, 2026
Merged

fix(vision): capture the functional slice_update return in DeepStack and MRoPE-section loops#666
inureyes merged 1 commit into
mainfrom
fix/issue-650-slice-update-injection

Conversation

@inureyes

@inureyes inureyes commented Jul 5, 2026

Copy link
Copy Markdown
Member

Summary

mlxcel_core::slice_update is functional (wraps mlx::core::slice_update, which returns a new array and never mutates src). Four loops called it as a bare statement and silently lost every write — the same class as the Granite 4 Vision blindness fixed by inject_at_positions in PR #649.

Site What was dropped Effect
src/models/qwen3_vl.rs (DeepStack loop) the whole per-layer visual refinement quality degradation (main features still enter via inputs_embeds)
src/models/qwen3_vl_moe.rs (DeepStack loop) same same
src/models/qwen2_vl.rs (apply_mrope) the H and W section writes every head dim kept temporal frequencies; image tokens carried no intra-image spatial position information
src/models/paddleocr_vl.rs (apply_mrope) same same

Fix

Capture the return. The DeepStack loops mirror the Granite fix: a contiguous single-image run collapses to one slice_update, disjoint runs fall back to per-row with the return captured. The apply_mrope sites capture the return of each section write.

Validation

  • Four co-located regression tests pin the semantics: deepstack_process must add visual embeds at masked positions (qwen3_vl, qwen3_vl_moe); apply_mrope must place per-axis frequencies into their sections (qwen2_vl, paddleocr_vl). All four fail on the pre-fix code.
  • Real checkpoints, left-red / right-blue split image through the CLI:
    • qwen2-vl-2b-4bit: "The color on the left side of the image is red, and the color on the right side is blue."
    • qwen3-vl-4b-instruct-4bit: "far left is red ... far right is blue."
    • qwen3-vl-30b-a3b-4bit (MoE): "the color on the left side is red, and the color on the right side is blue."
  • paddleocr_vl shares the identical fixed function with qwen2_vl and is covered by its regression test (no local checkpoint on the validation machine).
  • Existing qwen3_vl / qwen2_vl / paddleocr unit suites, cargo fmt, and cargo clippy --all-targets -- -D warnings are clean.

Closes #650

…and MRoPE-section loops

mlxcel_core::slice_update wraps the functional mlx::core::slice_update, which returns a new array and never mutates its src; four loops called it as a bare statement and silently lost every write (issue #650, the same class as the Granite 4 Vision blindness fixed by inject_at_positions). In qwen3_vl.rs and qwen3_vl_moe.rs the DeepStack injection loop dropped the whole per-layer visual refinement (the models ran on the main embeddings alone); the fix captures the return and, mirroring the Granite fix, collapses the contiguous single-image run to one slice_update with a per-row fallback for disjoint runs. In qwen2_vl.rs and paddleocr_vl.rs the apply_mrope section assembly dropped the H and W section writes, so every head dim kept the temporal frequencies and image tokens carried no intra-image spatial position information; the fix captures the return. Four co-located regression tests pin the semantics (deepstack_process must add visual embeds at masked positions; apply_mrope must place per-axis frequencies into their sections). Real-checkpoint validation: qwen2-vl-2b-4bit, qwen3-vl-4b-instruct-4bit, and qwen3-vl-30b-a3b-4bit all localize a left-red/right-blue split image correctly through the CLI; paddleocr_vl shares the identical fixed function with qwen2_vl and is covered by its regression test (no local checkpoint).
@inureyes inureyes added type:bug Bug fixes, error corrections, or issue resolutions area:models Model architectures, weights, loading, metadata priority:high High priority labels Jul 5, 2026
@inureyes inureyes merged commit dae509a into main Jul 5, 2026
5 checks passed
@inureyes inureyes deleted the fix/issue-650-slice-update-injection branch July 5, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:models Model architectures, weights, loading, metadata priority:high High priority type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(vision): captured slice_update in DeepStack/section-assembly injection loops (qwen3_vl, qwen3_vl_moe, qwen2_vl, paddleocr_vl)

1 participant