Skip to content

[Perf] minimax-h3: Skip attention for the packing's alignment rows in the Ulysses pipeline - #47566

Merged
mickqian merged 2 commits into
mainfrom
mick/h3-skip-pad-attention
Nov 1, 2026
Merged

mickqian merged 2 commits into
mainfrom
mick/h3-skip-pad-attention

Conversation

@mickqian

@mickqian mickqian commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

H3 pads its packed sequence to a multiple of 64 per Ulysses rank, so a 1280x704 request attends cu_seqlens [0, 33002, 33024]. cuDNN SDPA's varlen runs each segment as its own call, so every pipelined head group ran the real rows, a copy into the output, a cuDNN call for the 22 alignment rows and a second copy. With 7 groups per layer that is 28 extra launches and their host work per block.

Modifications

  • The alignment rows only ever reach their own rows (attention keeps them in their own segment, everything else is row-wise), so group attention now attends the real rows only, writes them straight into the output when the backend can (FA4), and zeroes the alignment rows. The real rows run the same call as before.
  • Test added to test_minimax_h3_dit_contract.py.

Accuracy Tests

Bit-identical output: MP4 md5 identical to main in every arm (4/4 on FA4, 4/4 forced-cuDNN).

Speed Tests and Profiling

4x B300 (stand-in for B200), same protocol as the three-waves PR; measured on top of it:

1280x704 1344x768
three-waves 26.55 s 31.78 s
+ this PR 26.46 s (-0.10) 31.63 s (-0.14)

Together with three-waves, the 20-prompt 720p median goes 26.793 -> 26.517 s (-1.03%).

Note: this touches the same H3 pipelined-Ulysses block in minimax_h3.py as the ROCm Ulysses-pipeline PR; whichever lands second needs a small rebase.

Checklist

  • Format your code according to the pre-commit config (isort / ruff / black pass).
  • Add unit tests (merged into the existing test file for the touched area).
  • Provide accuracy and speed benchmark results (above).

🤖 Generated with Claude Code


CI States

Latest PR Test (Base): ✅ Run #41207111782
Latest PR Test (Extra): ❌ Run #41207111700
Latest PR Test (AMD ROCm 10): ❌ Run #41207111822

… the Ulysses pipeline

H3 pads its packed sequence to a multiple of 64 per Ulysses rank, so a
1280x704 request attends cu_seqlens [0, 33002, 33024]. cuDNN SDPA's varlen
runs each segment as its own call, so every pipelined head group ran the
real rows (FA4 on sm_103, cuDNN on sm_100), a copy into the output, a
cuDNN call for the 22 alignment rows and a second copy: with 7 groups per
layer that is 28 extra launches and their host work per block.

The alignment rows only ever reach their own rows (attention keeps them in
their own segment, everything else is row-wise), so the group attention now
attends the real rows only, writes them straight into the output when the
backend can (FA4), and zeroes the alignment rows. The real rows run the
same call as before, so the output is bit-identical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@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 repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@mickqian

Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Oct 31, 2026
@mickqian
mickqian merged commit 5032f98 into main Nov 1, 2026
182 of 193 checks passed
@mickqian
mickqian deleted the mick/h3-skip-pad-attention branch November 1, 2026 02:03
@bingxche

bingxche commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

AMD CI jobs started failing after this PR was merged, and the evidence points at this change.

test_minimax_h3_dit_contract.py::test_group_attend_skips_only_the_trailing_alignment_rows, added by this PR, is skipped only when CUDA is unavailable; on ROCm it runs, falls back to the FlashAttention path and fails with ModuleNotFoundError: No module named 'flash_attn' (flash_attention_v3.py:239) on the rocm10 and rocm720 lanes.

The last passing run was on 8817f63c and the first failing run on cb0a7a95; this PR's commit 5032f988 is between them.

Failing jobs (2):

If this PR is the cause, a fix or a revert would turn these jobs green again. If it isn't, please say so here so the attribution can be corrected.

Posted automatically by amd-bot, the AMD CI monitor. Confidence: HIGH.

@bingxche

bingxche commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

@mickqian AMD CI jobs started failing after this PR was merged, and #47971 proposes a fix. Could you review it?

test_minimax_h3_dit_contract.py::test_group_attend_skips_only_the_trailing_alignment_rows, added by this PR, is gated only on torch.cuda.is_available(); on ROCm DynamicCudnnSDPAImpl falls back to the FlashAttention path and fails with No module named 'flash_attn' on rocm10, rocm720 and rocm724 in every run since it landed.

The last passing run was on 8817f63c and the first failing run on cb0a7a95; this PR's commit 5032f988 is between them.

Failing jobs (5):

If this PR isn't the cause, please say so here so the attribution can be corrected.

Posted automatically by amd-bot, the AMD CI monitor. Confidence: HIGH.

niehen6174 added a commit to niehen6174/sglang that referenced this pull request Nov 5, 2026
Reviewed the six upstream minimax_h3.py changes since the last pin
(sgl-project#46047, sgl-project#46709, sgl-project#47567, sgl-project#47569, sgl-project#47566 and the BCG output buffers). None
touches what h3_comfyui_v1 relies on in its single-GPU eager torch_sdpa
profile: DiTBlock.forward is unchanged, _attend still calls
fused_inplace_qknorm_rope on the _use_fused_qknorm_rope branch with the same
arguments (the new pipelined branch needs Ulysses, which the profile
rejects), and _embed still returns (embeddings, t_emb) in eager mode.
Yuxingwang-intel pushed a commit to Yuxingwang-intel/sglang that referenced this pull request Nov 6, 2026
…nt rows in the Ulysses pipeline (sgl-project#47566)

Co-authored-by: Mick Qian <mickqian@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diffusion SGLang Diffusion run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants