Repository navigation
[Perf] Fuse MiniMax H3 SwiGLU activation - #6283
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to be related to model: MinimaxH3. Model owners: @david6666666 @gcanlin, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Signed-off-by: Canlin Guo <canlinguosdu@gmail.com>
90432b7 to
14e919f
Compare
Signed-off-by: Canlin Guo <canlinguosdu@gmail.com>
| from vllm_omni.platforms import current_omni_platform | ||
|
|
||
|
|
||
| class SiluAndMul(CustomOp): |
There was a problem hiding this comment.
Verified from the code: as written, NPU keeps the native two-kernel path (forward_npu → forward_native), so Ascend behavior is unchanged — but a fused op already exists in-repo (torch_npu.npu_swiglu; see my comment on forward_npu) if you want the fusion to apply there too.
| self.op(out, x) | ||
| return out | ||
|
|
||
| def forward_npu(self, x: torch.Tensor) -> torch.Tensor: |
There was a problem hiding this comment.
NPU is left on the two-kernel native path even though a fused operator with this exact packed contract already exists in-repo: torch_npu.npu_swiglu(gate_up, dim=-1) (wrapped at vllm_omni/platforms/npu/models/minimax_h3.py:57, covered by test_minimax_h3_qwen3vl_swiglu.py at 2e-2 tolerance). RMSNorm.forward_npu (vllm_omni/diffusion/layers/norm.py:125) sets the lazy-import precedent:
def forward_npu(self, x):
import torch_npu
return torch_npu.npu_swiglu(x, dim=-1)With that the DiT MLP gets the same one-kernel activation on Ascend and the PR goal ("avoid the native two-kernel fallback") holds on NPU too.
| def __init__(self) -> None: | ||
| super().__init__() | ||
| if current_omni_platform.is_cuda() or current_omni_platform.is_rocm() or current_omni_platform.is_musa(): | ||
| self.op = torch.ops._C.silu_and_mul |
There was a problem hiding this comment.
Nit: self.op is resolved eagerly when the platform is cuda/rocm/musa, but the musa path never uses it (forward_musa → forward_cuda → x.device.type != "cuda" → native), and construction would crash on a musa build that doesn't register _C.silu_and_mul. Resolving the op lazily inside forward_cuda (or dropping is_musa() from the condition) avoids both.
| d = x.shape[-1] // 2 | ||
| output_shape = x.shape[:-1] + (d,) | ||
| out = torch.empty(output_shape, dtype=x.dtype, device=x.device) | ||
| self.op(out, x) |
There was a problem hiding this comment.
The byte-identical accuracy result surprises me: torch.ops._C.silu_and_mul computes silu in FP32 and rounds once to BF16, while the baseline F.silu(gate) * up materializes silu as BF16 before the multiply (double rounding), so I'd expect ULP-level divergence to propagate through 49 steps into the output bytes — the modulation PR (#6281) measures PSNR 39.55 dB from exactly this class of rounding change. Can you confirm how the comparison was produced (fused op active in both runs? same checkpoint/seed as the baseline)?
|
Both amd-ci (build 10690) and npu-ci (build 5498) are red on head while the CUDA/Intel lanes pass; logs are behind auth on my side. The npu lane doesn't run MiniMax H3 and the NPU path here is a pure fallback to the old eager math, so I can't correlate from the diff — could you check whether these are related or infra? |
Signed-off-by: Canlin Guo <canlinguosdu@gmail.com>
Signed-off-by: Canlin Guo <canlinguosdu@gmail.com>
This reverts commit 9683b7f.
) vllm-project#6283's DiT SwiGLU fusion wiring stays reverted (transformer call sites removed); this generic layer file is still required by the H3 VAE decoder ops, which are shared by every bench group and are not part of the fusion A/B under test.
Signed-off-by: Canlin Guo <canlinguosdu@gmail.com>
Signed-off-by: Canlin Guo <canlinguosdu@gmail.com>
Summary
This PR adds a local
SiluAndMullayer undervllm_omni.diffusion.layersand uses it in the MiniMax H3 MLP. The implementation preserves vLLM-Omni platform dispatch throughCustomOpandcurrent_omni_platform, while the CUDA path invokes the fusedtorch.ops._C.silu_and_mulkernel. This avoids depending on the vLLMSiluAndMulclass and prevents the diffusion configuration from falling back to the native two-kernel implementation.Performance
Benchmarked on MiniMax H3 Ref2VA with USP=4, eager execution, a 5-second output, and 49 denoising iterations. The existing main-branch result was reused as the baseline. The fused implementation was warmed up separately before both the 2-step profile and the 50-step end-to-end measurement.
End-to-End Performance
SiluAndMulSingle DiT Forward Profile
SiluAndMulThe profile confirms that every MiniMax H3 transformer layer dispatches to one
vllm::act_and_mul_kernelinstead of separate native SiLU and multiplication kernels. NCCL SendRecv time was 18.85 ms higher in the optimized profile due to run-to-run variation, partially masking the activation improvement in the overall DiT forward time.Accuracy
The optimized 50-step output is byte-for-byte identical to the baseline. Both files have the SHA-256 digest
bc383673714d4252467549335f01f92e52f26a4376a2f41aefd4e540c3c2021b.