Skip to content

[Bugfix][ROCm][Kimi-K3] Feed the DFlash drafter the AttnRes mixture instead of prefix sum - #56434

Open
tarik-sarac wants to merge 1 commit into
vllm-project:mainfrom
moonmath-ai:amd-aux-attn-res-stream
Open

tarik-sarac wants to merge 1 commit into
vllm-project:mainfrom
moonmath-ai:amd-aux-attn-res-stream

Conversation

@tarik-sarac

Copy link
Copy Markdown

Purpose

#50487 made the Kimi-K3 DFlash drafter read the pre-norm AttnRes mixture instead of
the running prefix sum, and added VLLM_KIMI_K3_AUX_ATTN_RES_STREAM to gate it. That
fix landed only in vllm/models/kimi_k3/nvidia/model.py. The ROCm implementation in
vllm/models/kimi_k3/amd/linear.py still taps the prefix sum and never reads the
flag, so setting it on AMD is a silent no-op.

Under AttnRes a layer only forwards the current block's running prefix; the committed
blocks live in the bank carried as residual. What the next consumer reads -- and what
the DFlash drafters were trained against -- is the pre-norm mixture over
bank[:num_blocks] + prefix. Tapping the prefix hands the drafter a tensor of the
right shape and dtype with the wrong value: nothing raises, acceptance simply drops.

The dispatch in vllm/models/kimi_k3/__init__.py is current_platform.is_rocm() with
no architecture gate, so gfx950 (CDNA4) is affected identically to gfx942.

This mirrors #50487's structure so the two paths stay comparable: the same config-time
rejection of an aux layer ending a non-final PP stage, the same one-shot log of which
layers are tapped and which mode is in force, the same fallback to the prefix when the
feature is off or the model is not AttnRes, and the same consumer-weight selection. It
reuses the existing AMD _apply_attn_res helper, which expands to the identical
attn_res(...) call the NVIDIA path makes. The capture is read-only beside the existing
tap -- it never writes back into hidden_states or residual, so the target's logits
are unchanged.

Test Plan

pytest tests/models/kimi_k3/test_amd_aux_attn_res_stream.py

New file, the AMD twin of tests/models/kimi_k3/test_aux_attn_res_stream.py. It asserts
the selection -- which weights and block count each tap uses -- rather than the mixture
itself, which is covered by test_amd_attn_res.py. Five cases:

  1. feature off via the flag -> returns the plain prefix, kernel not called
  2. feature on but the model is not AttnRes -> same fallback (the block size is what
    constructs the weights the lookups read)
  3. a tap with a consumer layer following -> uses layers[idx+1]'s norm/proj and its
    prev_valid_blocks
  4. the last layer on the final PP rank -> uses the model's output-side aggregation,
    counted the way forward() counts it
  5. an aux layer ending a non-final PP stage -> rejected at configuration time

End-to-end: Kimi-K3 at TP8 on 8xMI325X (gfx942), DSPARK gamma=3, fp8 KV,
--max-model-len 163840, prefix caching off, measured with a same-prompt differential
over identical corpus slices.

Test Result

before after
accepted tokens per step 2.35 2.86
decode throughput baseline +23%

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added kimi k3 rocm Related to AMD ROCm dflash bug Something isn't working labels Sep 11, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Sep 11, 2026
@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

…not the prefix sum

Signed-off-by: Tarik Sarac <tariksarac120@gmail.com>
@tarik-sarac
tarik-sarac force-pushed the amd-aux-attn-res-stream branch from d7a9662 to 9869304 Compare September 11, 2026 15:32
@tarik-sarac tarik-sarac changed the title [Bugfix][ROCm][Kimi-K3] Feed the DFlash drafter the AttnRes mixture, not the prefix sum [Bugfix][ROCm][Kimi-K3] Feed the DFlash drafter the AttnRes mixture instead of prefix sum Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dflash k3 kimi rocm Related to AMD ROCm

Projects

Status: Todo
Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant