Skip to content

[CI] [AMD] memory_pool: Keep separate K/V allocations on ROCm to fix Mooncake PD accuracy - #46875

Closed
michaelzhang-ai wants to merge 1 commit into
sgl-project:mainfrom
michaelzhang-ai:ci-fix/amd-disagg-unfused-kv
Closed

michaelzhang-ai wants to merge 1 commit into
sgl-project:mainfrom
michaelzhang-ai:ci-fix/amd-disagg-unfused-kv

Conversation

@michaelzhang-ai

@michaelzhang-ai michaelzhang-ai commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

Since #32272, MHATokenToKVPool stores each layer's K and V as two halves of one allocation. On ROCm this breaks Mooncake PD transfers: decode receives corrupted KV without any transfer error. In stage-b-test-large-8-gpu-mi35x-disaggregation-amd, test_disaggregation_basic.py (Qwen3-8B, Mooncake) dropped from 0.955 GSM8K accuracy to 0.0-0.01 on main (example job). NIXL and MORI tests in the same job are unaffected (NIXL GSM8K 0.765 before and after).

The last passing main run was at 5884e4f; the failures start with the commit range that includes #32272. The other candidates in that range don't touch this path for a bf16 Qwen3 PD run.

Modifications

Skip the fused K/V allocation on ROCm (and not _is_hip) and keep separate per-layer K and V buffers, as before #32272. The fused layout serves the CUDA trtllm fmha_v2 paged kernels, which don't run on ROCm. Other platforms are unchanged.

Test plan

  • stage-b-test-large-8-gpu-mi35x-disaggregation-amd on rocm10, dispatched on this commit: all four files pass. test_disaggregation_basic.py GSM8K 0.950 / 0.965, test_disaggregation_pp.py 0.945-0.955, NIXL 0.770, MORI OK (job).

CI States

Latest PR Test (Base): ❌ Run #40555506148
Latest PR Test (Extra): ❌ Run #40555506102
Latest PR Test (AMD ROCm 10): ✅ Run #40555506196

sgl-project#32272 stores K and V as halves of one allocation per layer. On ROCm,
Mooncake PD transfers then hand decode corrupted KV: Qwen3-8B GSM8K in
test_disaggregation_basic.py drops from 0.955 to ~0.0 with no transfer
errors, while NIXL and MORI are unaffected. The fused layout only serves
the CUDA trtllm fmha_v2 kernels, so ROCm keeps separate buffers.
@michaelzhang-ai
michaelzhang-ai marked this pull request as ready for review October 27, 2026 17:58
@michaelzhang-ai michaelzhang-ai added the run-ci CI: run the baseline test suite on this PR label Oct 27, 2026
@michaelzhang-ai
michaelzhang-ai requested a review from Lzy17 October 27, 2026 17:58
@github-actions github-actions Bot added memory-pool and removed run-ci CI: run the baseline test suite on this PR labels Oct 27, 2026
@michaelzhang-ai michaelzhang-ai added the run-ci CI: run the baseline test suite on this PR label Oct 27, 2026
@omirosh

omirosh commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Hi @michaelzhang-ai , looks like a duplicate for #46861

@1am9trash

1am9trash commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Hi @michaelzhang-ai,

The code change here is the same as #46861. Both add a not _is_hip guard.
Since #46861 was opened earlier and also includes a unit test. I think we can close this PR and continue on that one?
Thanks.

@michaelzhang-ai

Copy link
Copy Markdown
Collaborator Author

Closing as a duplicate of #46861, which made the same change and is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

memory-pool run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants