perf(qwen3-14b TQ decode): rotated-space attention, drop per-step KV …#773
perf(qwen3-14b TQ decode): rotated-space attention, drop per-step KV …#773sunghajung6688 wants to merge 1 commit into
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
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:
📝 WalkthroughWalkthroughChangesRotated attention decode
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DecodeLayer
participant RotatedDequant
participant QKAndSV
participant OnlineSoftmax
participant Writeback
DecodeLayer->>DecodeLayer: rotate padded Q by R
DecodeLayer->>RotatedDequant: dequantize K and V into low/high halves
RotatedDequant->>QKAndSV: rotated K halves
QKAndSV->>OnlineSoftmax: summed QK scores and split value products
OnlineSoftmax->>Writeback: normalized low/high context halves
Writeback->>DecodeLayer: unrotate by R^T
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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@models/qwen3/14b/qwen3_14b_decode_tq_draft.py`:
- Around line 518-532: Update the attention writeback block around ctx_unrot and
attn_out so only the first Q_HEAD_BATCH query rows are flattened and assembled.
Slice or otherwise restrict ctx_unrot before reshape, preserving the existing
unrotation while preventing padded rows from being written.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a77419c9-a227-44a0-83c8-f1d13805aace
📒 Files selected for processing (2)
models/qwen3/14b/qwen3_14b_decode_tq_draft.pymodels/qwen3/14b/turboquant_kv.py
| # Finalize: ctx = oi / li (rotated), unrotate once (·R^T), write to attn_out. | ||
| with pl.at(level=pl.Level.CORE_GROUP, name_hint="attention_writeback"): | ||
| ctx = pl.row_expand_div(oi, li) | ||
| ctx_lo = pl.row_expand_div(oi_lo, li) | ||
| ctx_hi = pl.row_expand_div(oi_hi, li) | ||
| unrot_r_lo = pl.slice(rot_slice, [HEAD_DIM, HALF_DIM], [0, 0]) | ||
| unrot_r_hi = pl.slice(rot_slice, [HEAD_DIM, HALF_DIM], [0, HALF_DIM]) | ||
| ctx_unrot = pl.add( | ||
| pl.matmul(pl.cast(ctx_lo, target_type=pl.BF16), unrot_r_lo, b_trans=True, out_dtype=pl.FP32), | ||
| pl.matmul(pl.cast(ctx_hi, target_type=pl.BF16), unrot_r_hi, b_trans=True, out_dtype=pl.FP32), | ||
| ) | ||
| ctx_flat_bf16 = pl.cast( | ||
| pl.reshape(ctx, [1, Q_HEAD_PAD * HEAD_DIM]), | ||
| pl.reshape(ctx_unrot, [1, Q_HEAD_PAD * HEAD_DIM]), | ||
| target_type=pl.BF16, | ||
| ) | ||
| attn_out = pl.assemble(attn_out, ctx_flat_bf16, [b, q_base * HEAD_DIM]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Write back only the non-padded query rows.
ctx_unrot contains Q_HEAD_PAD rows, but this group owns only Q_HEAD_BATCH heads. Flattening every padded row can overwrite the following group’s output or exceed attn_out for the final group.
Proposed fix
ctx_unrot = pl.add(
pl.matmul(pl.cast(ctx_lo, target_type=pl.BF16), unrot_r_lo, b_trans=True, out_dtype=pl.FP32),
pl.matmul(pl.cast(ctx_hi, target_type=pl.BF16), unrot_r_hi, b_trans=True, out_dtype=pl.FP32),
)
+ ctx_valid = pl.slice(
+ ctx_unrot, [Q_HEAD_BATCH, HEAD_DIM], [0, 0],
+ )
ctx_flat_bf16 = pl.cast(
- pl.reshape(ctx_unrot, [1, Q_HEAD_PAD * HEAD_DIM]),
+ pl.reshape(ctx_valid, [1, Q_HEAD_BATCH * HEAD_DIM]),
target_type=pl.BF16,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Finalize: ctx = oi / li (rotated), unrotate once (·R^T), write to attn_out. | |
| with pl.at(level=pl.Level.CORE_GROUP, name_hint="attention_writeback"): | |
| ctx = pl.row_expand_div(oi, li) | |
| ctx_lo = pl.row_expand_div(oi_lo, li) | |
| ctx_hi = pl.row_expand_div(oi_hi, li) | |
| unrot_r_lo = pl.slice(rot_slice, [HEAD_DIM, HALF_DIM], [0, 0]) | |
| unrot_r_hi = pl.slice(rot_slice, [HEAD_DIM, HALF_DIM], [0, HALF_DIM]) | |
| ctx_unrot = pl.add( | |
| pl.matmul(pl.cast(ctx_lo, target_type=pl.BF16), unrot_r_lo, b_trans=True, out_dtype=pl.FP32), | |
| pl.matmul(pl.cast(ctx_hi, target_type=pl.BF16), unrot_r_hi, b_trans=True, out_dtype=pl.FP32), | |
| ) | |
| ctx_flat_bf16 = pl.cast( | |
| pl.reshape(ctx, [1, Q_HEAD_PAD * HEAD_DIM]), | |
| pl.reshape(ctx_unrot, [1, Q_HEAD_PAD * HEAD_DIM]), | |
| target_type=pl.BF16, | |
| ) | |
| attn_out = pl.assemble(attn_out, ctx_flat_bf16, [b, q_base * HEAD_DIM]) | |
| # Finalize: ctx = oi / li (rotated), unrotate once (·R^T), write to attn_out. | |
| with pl.at(level=pl.Level.CORE_GROUP, name_hint="attention_writeback"): | |
| ctx_lo = pl.row_expand_div(oi_lo, li) | |
| ctx_hi = pl.row_expand_div(oi_hi, li) | |
| unrot_r_lo = pl.slice(rot_slice, [HEAD_DIM, HALF_DIM], [0, 0]) | |
| unrot_r_hi = pl.slice(rot_slice, [HEAD_DIM, HALF_DIM], [0, HALF_DIM]) | |
| ctx_unrot = pl.add( | |
| pl.matmul(pl.cast(ctx_lo, target_type=pl.BF16), unrot_r_lo, b_trans=True, out_dtype=pl.FP32), | |
| pl.matmul(pl.cast(ctx_hi, target_type=pl.BF16), unrot_r_hi, b_trans=True, out_dtype=pl.FP32), | |
| ) | |
| ctx_valid = pl.slice( | |
| ctx_unrot, [Q_HEAD_BATCH, HEAD_DIM], [0, 0], | |
| ) | |
| ctx_flat_bf16 = pl.cast( | |
| pl.reshape(ctx_valid, [1, Q_HEAD_BATCH * HEAD_DIM]), | |
| target_type=pl.BF16, | |
| ) | |
| attn_out = pl.assemble(attn_out, ctx_flat_bf16, [b, q_base * HEAD_DIM]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@models/qwen3/14b/qwen3_14b_decode_tq_draft.py` around lines 518 - 532, Update
the attention writeback block around ctx_unrot and attn_out so only the first
Q_HEAD_BATCH query rows are flattened and assembled. Slice or otherwise restrict
ctx_unrot before reshape, preserving the existing unrotation while preventing
padded rows from being written.
840c145 to
1cec3a6
Compare
3f5045f to
ca6addc
Compare
… attention (MAX_SEQ=4096) The draft prefill hardcoded MAX_SEQ=128, sizing rope_cos/sin to [128, HEAD_DIM]. Attention reads rope_cos[pos] up to seq_len, so prompts >128 tokens read OOB rope values (clamped on device, no fault) -> wrong RoPE -> garbage logits. Serving showed correct output <=128 tokens and pure garbage above the ~128 cutoff. - MAX_SEQ: 128 -> M.max_seq (4096); rope tables now cover full prompt length. - Rewrite attention to chunk-streaming, mirroring prefill_fwd_a8w8 (its lines 433-540): one for-sb0 chunk loop with per-chunk [SB_BATCH*...] scratch allocated inside (liveness scoped per chunk), replacing [MAX_CTX_BLOCKS*...] buffers kept live across four separate full-range loops. Scratch is now sized by the constant SB_BATCH, independent of prompt length (no short-prompt padding to worst-case). - SB_BATCH: 128 -> 32 (matches prefill_fwd_a8w8). - Preserve causal valid_shape + fillpad(min) masking per lane. - Remove now-dead MAX_CTX_BLOCKS. Compile-verified on a2a3sim. Runtime correctness pending device validation (the sim runtime assert is pre-existing for these TQ kernels, not a regression). The scale 1/16 KV scale-write bug in turboquant_kv.py remains open (separate issue). Co-Authored-By: Claude <noreply@anthropic.com>
ca6addc to
613c80a
Compare
The TQ decode dequant unrotated the entire KV cache every step (O(seq_len x head_dim^2) per step per layer), making long-sequence decode exceed the stream-sync timeout. Move the inverse rotation out of the per-row dequant into the cheap single-token Q (q.R) and the single attention output (.R^T): the paged cache stays rotated (K.R / V.R), and QK = (q.R).(K.R)^T = q.K^T (R orthogonal cancels). Precision unchanged.