Skip to content

fix(ep/intranode training): restore replay-mode payload dispatch dropped by #532 - #675

Open
sudhu2k wants to merge 3 commits into
ROCm:mainfrom
sudhu2k:sudhu/mori_main_replay_fix
Open

sudhu2k wants to merge 3 commits into
ROCm:mainfrom
sudhu2k:sudhu/mori_main_replay_fix

Conversation

@sudhu2k

@sudhu2k sudhu2k commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Motivation

Restore correctness of replay-mode dispatch, which is used in MoE training
backward pass. Since #532 (ae9635f7, "feat(ep/intranode): bring MoE dispatch/combine
up on gfx1250 (MI450)"
), intra-node replay-mode dispatch moved no payload, so the
scattered activations/gradients were left stale. In Megatron-LM MoE training this surfaced
as an exploding gradient norm and immediate divergence,
while the forward loss stayed exactly correct.

The forward pass was unaffected because it uses cache-routing dispatch + replay-routing
combine
(both intact). Only the backward pass uses replay-routing dispatch
(MoriCombine.backwardop.dispatch(routing=...)), which was the single broken path.

Technical Details

#532 refactored EpDispatchIntraNodeKernel_body (src/ops/dispatch_combine/intranode.hpp)
into three block-cooperative phases (count tokens → reserve slots → scatter payload), and
gated all three on if (args.tokenIndices && args.inpTokenBuf && !args.replayMode).
This inadvertently dropped the pre-#532 else branch that handled replay routing, so in
replay mode the kernel skipped every phase and moved zero payload into dispatchOut
(grad_x stayed stale).

Fix: re-add the replay branch. In replay mode we recover (destPe, destTokId) from the
dispDestTokIdMap written by the matching cache-routing dispatch, then copy metadata
(indices, weights, scales) and the token payload — exactly the pre-#532 behavior.
destPeTokenCounter / dispTokOffset are intentionally left untouched: in replay mode the
host takes total_recv from the routing handle rather than the kernel's signal, and the
completion handshake is a pure ordering barrier.

  • Scope: intra-node kernel only.

Why existing tests missed it

test_replay_correctness exercised replay-dispatch but only asserted routing-map
immutability and the recv token count. A replay dispatch that moves zero payload leaves
the routing maps untouched and preserves the count, so all assertions passed while no
activation was scattered.

Regression guard added: after the replay dispatch, route the same payload through a
fresh op on the fully independent default path (no routing handle) and compare downstream
combine outputs. Combine outputs are in source-token order, so the check is invariant to
the recv-slot assignment that differs across ops; a broken replay (stale/zero payload)
diverges by ~activation magnitude and trips the assertion.

Files changed

  • src/ops/dispatch_combine/intranode.hpp — restore replay-mode payload dispatch.
  • tests/python/ops/test_dispatch_combine_routing_handle.py — payload guard in
    _replay_correctness (covers IntraNode + InterNodeV1, single- and two-node).

Test Plan

  • Unit: pytest tests/python/ops/test_dispatch_combine_routing_handle.py -k replay_correctness
    on 8 GPUs, covering:
    • test_replay_correctness[intra] — IntraNode.
    • test_replay_correctness[v1] — InterNodeV1, single-node.
    • test_replay_correctness_v1_two_nodes — InterNodeV1, 2-node RDMA (gpu_per_node=4).

Submission Checklist

sudhu2k and others added 3 commits September 15, 2026 23:04
…ernel

The gfx1250 bring-up split cache-routing dispatch into three non-replay phases and removed the replay branch, so MoriCombine.backward's replay dispatch sent no payload, leaving grad_x stale and causing exploding grad norms. Re-add the replay path: read (destPe, destTokId) back from the cached dispDestTokIdMap and scatter weights, indices, scales, and tokens. Token counters are intentionally untouched because the host derives total_recv from the routing handle in replay mode.
The replay-correctness test only checked routing-map immutability and the recv
token COUNT, so the ROCm#532 regression that dropped the replay branch of
EpDispatchIntraNodeKernel_body (replay dispatch moved zero payload) still passed.

Route the same grad payload through a fresh cache-routing op and compare
downstream combine outputs (source-token order, invariant to recv-slot
assignment). Fails on the buggy kernel, passes with the restored replay branch.

Co-authored-by: Cursor <cursoragent@cursor.com>
The cache-routing reference op in the replay-correctness payload guard still
relied on a routing handle, making it vulnerable to the same class of bugs as
the replay path. Switch the reference to the fully independent default path:
default dispatch populates the op-owned routing maps, so the combine reads them
directly and serves as a bug-independent oracle for the scattered payload.
@sudhu2k
sudhu2k requested a review from TianDi101 September 16, 2026 15:41
@sudhu2k sudhu2k self-assigned this Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant