Refactor Qwen3 decode forward path#670
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR propagates FP32 hidden states through the LM-head computation across Qwen3-14B decode and prefill kernels, removing BF16 casts before rms_lm_head. It also refactors decode_fwd's token embedding and greedy sampling to use pl.spmd, removes token_embed.py and qwen3_14b_l3_generate.py, and updates docs/tests/comments from decode_layer.py to decode_fwd.py. ChangesFP32 carry into rms_lm_head
decode_fwd SPMD refactor and file cleanup
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors the Qwen3-14B generation pipeline by transitioning the final RMSNorm and LM-head inputs to FP32, which removes redundant BF16 casting and round-trips. Additionally, it cleans up the codebase by deleting several standalone files (greedy_sample.py, qwen3_14b_generate_l3.py, and token_embed.py) in favor of inline implementations, updates documentation and tests, and adopts pl.spmd in inline functions. The review feedback suggests a valuable refactoring to eliminate duplicated loop logic when scanning winning logits in decode_fwd.py, along with a corresponding update to the test assertions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
c5c1b12 to
3d8c338
Compare
- Rename the Qwen3 fused decode entry to decode_fwd.py - Keep token embedding and greedy sampling on the inline SPMD path - Feed final RMSNorm from the FP32 carry before the LM head - Remove the obsolete standalone L3 generate entry
…om CI The TurboQuant reference kernels qwen3_14b_decode_tq.py and qwen3_14b_prefill_tq.py hit a pre-existing, device/occupancy-sensitive AICore 507018 drain timeout (and a ring task-allocator heap-exhaustion deadlock on *sim) unrelated to this decode refactor: the files are byte-identical to main here (only trailing blank lines were touched) and main's own a2a3 job passes them intermittently. Rename both to the `_draft.py` convention so `.github/scripts/detect_changes.py` drops them from the sim / a2a3 selection until the runtime issue is resolved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d5c3320 to
448e22f
Compare
This reverts commit be52aab.
Summary
Related Issues