Repository navigation
[AMD][PD] enable mori transfer for PD decode DCP - #37358
Conversation
Signed-off-by: inkcherry <mingzhi.liu@amd.com> # Conflicts: # python/sglang/srt/disaggregation/mori/conn.py
Signed-off-by: inkcherry <mingzhi.liu@amd.com>
| requires_dcp_relayout = getattr( | ||
| req.disagg_kv_sender, "requires_dcp_relayout", None | ||
| ) | ||
| if requires_dcp_relayout is not None and requires_dcp_relayout(): | ||
| return | ||
|
|
There was a problem hiding this comment.
QQ: Does only Mori need this? If so, I think this introduces another code path divergence. Also, I think we should avoid using getattr. And the code here is confusing.
There was a problem hiding this comment.
Thanks, yes, writing it that way does introduce some confusion.
#32837 — this PR supports further optimization of DCP. But it only covers Mooncake and NIXL.
-
The current PR first supports the basic functionality of DCP for mori. Now I modified it here with an API
supports_cached_prefix_early_sendto bypass ·. No changes for nixl and mooncake. -
In subsequent separate PRs, we will try to leverage feat: support Kimi Linear PD disaggregation with DCP #32837 and the validated optimizations. If it works, we can remove this bypass.
There was a problem hiding this comment.
re-verified on Kimi3 E2E.
ShangmingCai
left a comment
There was a problem hiding this comment.
No other comment, others look good. But before we merge, we need to discuss about the diff in prefill.py.
Signed-off-by: inkcherry <mingzhi.liu@amd.com>
Thank you, I have replied with the reason and revised the code. Do you think we should directly support the mori layout-related work through this PR? I am inclined to separate it. |
Signed-off-by: inkcherry <mingzhi.liu@amd.com>
Signed-off-by: inkcherry <mingzhi.liu@amd.com>
Signed-off-by: inkcherry <mingzhi.liu@amd.com>
|
@ShangmingCai Applied the DCP KV layout optimization to Mori without introducing backend-specific control-flow divergence now. |
|
/tag-and-reurn-ci |
|
⛔ See the command reference. |
|
/tag-and-rerun-ci |
|
@Lzy17 please have a review. |
|
The port itself looks right -- every helper this calls ( One gap: nothing in CI executes the new path. Two smaller things:
|
modified. |
There was a problem hiding this comment.
Thanks for the PR. Items:
- Add the DCP e2e (small MLA model, prefill dcp1 / decode dcp2) and a test that mori is accepted, per my earlier comment. Neither is in the current head.
- State in the description, or split out, that this also changes non-DCP paths (layer-id PP mapping, mamba TP-slice), which only mocked tests cover.
- Bounds-check
state_conv_shard_groups[i]/state_slice_outer_counts[i]insend_state, as is done forsrc_dims. - Surface
requires_dcp_relayouterrors from_add_remote_peeras a request failure. The caller only logs, so the request waits for its timeout. - Coordinate with #41769 (same
_send_swa_dsa_stateloop; dst-length lookups must usej) and #41140 (registration message needs the four new fields).
|
/rerun-failed-ci |
Motivation
PD decode with DCP currently rejects the Mori transfer backend because Mori does not implement the token relayout required by round-robin DCP KV shards.
Changes
Validation
The Kimi-K3 E2E validation used an AITER DCP-enabled stack; this PR only changes the Mori transfer path.
CI States
Latest PR Test (Base): ✅ Run #38288271653
Latest PR Test (Extra): ❌ Run #38288270583
Latest PR Test (AMD ROCm 10): ❌ Run #38288271415