[PyT] [Common] add support for enabling cuda graph under thd format in megatron.#2898
Conversation
Greptile SummaryThis PR enables CUDA Graph capture under THD format in Megatron by eliminating two root causes: GPU→CPU syncs triggered by
Confidence Score: 5/5The changes are well-scoped and the core graph-safety fixes are correct; the only nits are efficiency issues in the non-fused attention backward where zero-fills run redundantly. All graph-safety invariants are correctly maintained: arange-based masks stay on-device, the device-tensor coefficient path never reads the scalar on the host, and the existing eager-mode fill_ paths are preserved. The new second zero-fill block in the CP backward is redundant for the non-fused path and skips the is_graph_capturing() branch that would give cheaper fill_ in eager mode, but neither issue affects correctness. transformer_engine/pytorch/attention/dot_product_attention/context_parallel.py — the new unconditional arange-mask zero-fill block after nvtx_range_pop runs twice in the non-fused attention THD path and lacks the is_graph_capturing() branch used by the existing block above it. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Backward: AttnFuncWithCPAndKVP2P] --> B{qkv_format == 'thd' AND\nnot use_fused_attention AND\nboth padded seqlens non-None?}
B -- Yes --> C{is_graph_capturing?}
B -- No --> E
C -- Yes --> D1[arange mask zero-fill dq/dk/dv]
C -- No --> D2[fill_ zero-fill dq/dk/dv]
D1 --> E[FP8 quantize if needed]
D2 --> E
E --> F[A2A communicate if cp_size_a2a > 1]
F --> G{qkv_format == 'thd'?}
G -- Yes --> H[arange mask zero-fill always\ndq / dk / dv independently guarded]
G -- No --> I[return dq/dk/dv]
H --> I
style H fill:#ffe0b2
style B fill:#e3f2fd
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Backward: AttnFuncWithCPAndKVP2P] --> B{qkv_format == 'thd' AND\nnot use_fused_attention AND\nboth padded seqlens non-None?}
B -- Yes --> C{is_graph_capturing?}
B -- No --> E
C -- Yes --> D1[arange mask zero-fill dq/dk/dv]
C -- No --> D2[fill_ zero-fill dq/dk/dv]
D1 --> E[FP8 quantize if needed]
D2 --> E
E --> F[A2A communicate if cp_size_a2a > 1]
F --> G{qkv_format == 'thd'?}
G -- Yes --> H[arange mask zero-fill always\ndq / dk / dv independently guarded]
G -- No --> I[return dq/dk/dv]
H --> I
style H fill:#ffe0b2
style B fill:#e3f2fd
Reviews (11): Last reviewed commit: "[pre-commit.ci] auto fixes from pre-comm..." | Re-trigger Greptile |
| _aT_fwd = cu_seqlens_q[-1] | ||
| if _out.shape[0] > 0: | ||
| _m_fwd = torch.arange(_out.shape[0], device=_out.device) >= _aT_fwd | ||
| _out[_m_fwd] = 0 |
There was a problem hiding this comment.
Do you only have pad tokens at the end of the batch? Would the pads between sequences in the batch also need to be zeroed out?
There was a problem hiding this comment.
Yes, Megatron PR #4359 only adds tail padding to the packed THD buffer. These zero-fills only clear positions >= cu_seqlens_*_padded[-1]; inter-sequence padding stays with the existing TE handling.
There was a problem hiding this comment.
Tail padding is probably a very special case of pad_between_seqs=True. If we want to claim that THD + CUDA graph is working, could we please zero out all the inter-sequence padding as well (here and in a few other places)? Thanks!
There was a problem hiding this comment.
In current megatron thd e2e test, due to accuracy problem, we let cu_seqlens_* to be equal to cu_seqlens_*_padded, and address the inter-sequence padding by loss mask. So here we want to only focus on tail padding.
Do you know if these tests were running with FlashAttention backend or FusedAttention backend? |
ff1daaf to
851d082
Compare
The Megatron E2E bitwise tests run with FusedAttention, not FlashAttention. In the Qwen3 THD config, padded cu_seqlens make TE infer The unit tests cover the backends separately: |
Signed-off-by: HaochenYuan <haocheny@nvidia.com>
0f32a09 to
2c58118
Compare
Signed-off-by: HaochenYuan <haocheny@nvidia.com>
b2358ab to
b6aa597
Compare
Signed-off-by: HaochenYuan <haocheny@nvidia.com>
|
/te-ci pytorch L1 |
|
The remaining CI failure looks like an infra/runner issue?: GitHub reports "The hosted runner lost communication with the server" during the Build step. |
The failed tests are due to a different issue which we're fixing, so I think you can ignore those. Thanks! |
a5a605b to
f003c5a
Compare
|
Add one fuesd_moe_aux_loss kernel in this PR, the only change is allow the total_num_tokens to be a tensor type, so when we enable router fusion in megatron with cuda graph in thd format, the total_num_tokens can vary across different micro-batch. The previous int type couldn't allow value change when replay graph. |
Signed-off-by: HaochenYuan <haocheny@nvidia.com>
Signed-off-by: HaochenYuan <haocheny@nvidia.com>
80b4e86 to
9eb153a
Compare
Signed-off-by: HaochenYuan <haocheny@nvidia.com>
9eb153a to
cee7b6a
Compare
for more information, see https://pre-commit.ci
Signed-off-by: HaochenYuan <haocheny@nvidia.com>
0cdff8a to
bce6bc5
Compare
|
/te-ci L1 |
Co-authored-by: Tim Moon <4406448+timmoon10@users.noreply.github.com> Signed-off-by: Tim Moon <4406448+timmoon10@users.noreply.github.com>
for more information, see https://pre-commit.ci
|
/te-ci L1 |
Description
This PR supports enabling cuda graph under thd format in megatron.
Related Megatron PR 4359.
Two root causes block THD under CUDA Graph capture:
torch.equal()on CUDA tensors andcu_seqlens[-1]as a Python int both trigger implicit device sync.padded slots undefined; in MLA + CP these survive into gradient accumulation.
Patches
attention/dot_product_attention/dot_product_attention.pytorch.equal()check; forcepad_between_seqs=Truefor THD (always safe).attention/dot_product_attention/context_parallel.py(AttnFuncWithCPAndKVP2P.forward)out/out_retpastcu_seqlens_q_padded[-1]//cp_sizeafter CP assembly.arange-based mask, no sync.attention/dot_product_attention/context_parallel.py(AttnFuncWithCPAndKVP2P.backward, existing zero-fill)is_current_stream_capturing(), usedq.shape[0]instead ofcu_seqlens[-1]for the slice end.attention/dot_product_attention/context_parallel.py(AttnFuncWithCPAndKVP2P.backward, before return)dq/dk/dvpastcu_seqlens_q_padded[-1]//cp_sizeviaarangemask.attention/dot_product_attention/backends.py(FusedAttnFunc.backward)empty_like→zeros_likefordq/dk/dvso MLA+CP path doesn't leak garbage.cpp_extensions/fused_attn.py(fused_attn_fwd/fused_attn_bwd)cu_seqlens_q[-1]fort3hd/th3d/thd_*layouts.arangemask.All zero-fills use
torch.arange(N, device=...) >= boundso the bound stays a CUDAtensor — graph-safe.
Validation
Both Moonlight-16B (MLA + MoE, CP=2) and Qwen3-8B (GQA, CP=2) produce noGraph vs
cudaGraph bitwise identical results in
--deterministic-modeagainst this branch. See details in test.Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: