[ROCm][MoE] Pad the AITER MoE intermediate size at allocation time, and round the expert-group count to a kernel that exists - #55368
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
eca031f to
b619cb1
Compare
b619cb1 to
837ae11
Compare
|
@sshlyapn will following the MoE tuning guide (online tune Line where ONLINE TUNE feature exists. |
Thank you for comment! Unfortunately no - tuning can't fix this, because the tile width isn't a tuned parameter. CK instance is selected from inter_dim directly (inter_dim <= 192 then 64-wide, else 128-wide), in both stage-1 and stage-2 dispatch, and CK requires inter_dim to be divisible by whichever N tile it picks, so no tuned config can land inter_dim=224 - IsSupportedArgument rejects it and we get "device_gemm ... does not support this GEMM problem". It's a kernel implementation requirement and it's scoped to the bf16/fp16 data types only. |
|
Hi @tjtanaa @AndreasKaratzas could you please take a look at this PR when you have a moment? |
simondanielsson
left a comment
There was a problem hiding this comment.
This looks similar to #55251, can we adopt the same type of changes?
This also emphasizes that perhaps we should fix this on the aiter side instead. WDYT?
|
Allocation-time pad + zero tails is the right place to fix the CK shape reject, and it avoids the bias-length bug of a post-load rebuild. Please address before merge:
|
|
@simondanielsson @ChuanLi1101, thanks for the comments!
Yeah, the changes are really close, but they address different data types: unquantized BF16/FP16 vs FP8 in the mentioned PR. The logic itself looks the same, I just preferred not to zero out the buffers when the hidden dimension is also padded, as that's not what the MoE triggers/requires - so I'd prefer to keep the scope of the changes narrower
Agree, the better fix would definitely be improved unaligned-shape support in AITER. I've created a ticket to track the issue from AITER side: ROCm/aiter#5444. Meanwhile, since that might take some time, I'd suggest keeping this padding to provide functional support for models where such unaligned shapes might appear
Corresponding test has been added, thanks!
Absolutely agree. Here is the AITER issue for further improvements in this direction: ROCm/aiter#5444
Done |
f7f97b8 to
99b3b6f
Compare
AITER's CK 2stages MoE kernel dispatches on inter_dim <= 192: below it both
stages use 64-wide tiles, above it at least one stage uses a 128-wide tile.
CK's IsSupportedArgument rejects an intermediate size not divisible by that
width, so TP splits like 1792/8=224 fail with "device_gemm ... does not
support this GEMM problem".
Round the per-partition intermediate up in maybe_roundup_sizes instead of
rebuilding the weights after loading. The hook runs before create_weights,
so the padded size reaches allocation and the loader (which derives shard
offsets from the checkpoint) fills the real rows.
Fixes two defects in the post-load approach:
- w13_bias/w2_bias were sized off the unpadded intermediate, leaving the
bias shorter than the weight it is added to when has_bias is set.
- layer.intermediate_size_per_partition kept the pre-pad value while
moe_config was updated, desyncing the two.
It also drops a duplicate allocation of w13/w2 per layer at load.
Allocating padded means the loader never writes the tail, so w13/w2 are
zero-initialized when the dim is padded; the pad lanes must be inert
(silu(0) * 0 = 0, matching the zero columns in w2).
Mirror AITER's threshold rather than always aligning to 128: inter_dim 192
is unaligned but valid, and inter_dim is a tuned-config lookup key. Across
the 148 shipped AITER config files (11279 rows) the rule pads no tuned row;
a flat 128 would move 207 of them off their tuned entry.
Gated on UnquantizedMoeBackend.AITER, only selected on ROCm with AITER MoE
enabled. The rule is dtype-independent because CK derives both tile widths
from sizeof(A0DataType), identical for fp16 and bf16, and the kernels
TORCH_CHECK that the output dtype is one of those two.
Tested on vllm/vllm-openai-rocm:nightly-8a728663c (gfx950):
pytest tests/kernels/moe/test_rocm_aiter_moe.py -> 64 passed
Signed-off-by: Sergei Shliapnikov <sergei.shliapnikov@amd.com>
99b3b6f to
409c7f7
Compare
Purpose
Two independent startup failures on the ROCm AITER MoE path, both caused by shapes AITER has no compiled kernel for, both focusing on IFM/K2-Horizon-375B-A23B model support.
1. Unaligned per-partition intermediate size - unquantized path only, fp8 AITER alignment is unchanged
AITER's CK 2stages MoE kernel dispatches on
inter_dim <= 192: below it both stages use 64-wide tiles, above it at least one stage uses a 128-wide tile. CK'sIsSupportedArgumentrejects an intermediate not divisible by that width, somoe_intermediate_size = 1792at TP=8 gives224and fails withdevice_gemm ... does not support this GEMM problem.Fix: round the intermediate up in
maybe_roundup_sizes, the existing hook for this (already overridden by five other quant methods). It runs beforecreate_weights, so the padded size reaches allocation and the loader — which derives shard offsets from the checkpoint, not the parameter shape — fills only the real rows.Alignment rule:
64 if inter_dim <= 192 else 128, mirroring AITER's own threshold rather than always using 128. A flat 128 mispredicts 2 of the 19 sizes measured against the real kernel (64 and 192 both pass despite not being 128-aligned) — it is a wrong model that happens to be safe. It also costs:inter_dimis a tuned-config lookup key, and across the 148 shipped AITER config files (11279 rows) the threshold rule pads 0 tuned rows while a flat 128 would move 207 off theirs onto the heuristic fallback. Rounding up never crosses the boundary (96->128,160->192,192->192all stay<= 192), so the alignment picked is always the one the dispatcher then applies. Dtype-independent: CK derives both tile widths fromsizeof(A0DataType), identical for fp16 and bf16.Zero-init. The loader narrows to the checkpoint extent and never writes the tail, so
w13/w2are allocated withtorch.zeroswhen either dim is padded — unpadded layers keeptorch.emptyand pay nothing. Pad lanes are then bit-exactly inert:silu(0) * 0 = 0, and the zero columns inw2contribute exactly zero to stage 2. Withtorch.emptythe tail holds recycled device memory, which is live weight to the kernel; whether that hurts depends on load-time allocation order, so it presents as sporadic implausible tokens with no error. mxfp4 and quark nvfp4 already allocate zeroed; #55251 is doing the same for fp8.Scope. Gated on
UnquantizedMoeBackend.AITER, only selected on ROCm with AITER MoE enabled; every other unquantized backend falls through the base hook untouched. The quantized AITER paths have their own overrides with different constants and are not touched — fp8 needs 256.Known interaction. Since
intermediate_size_per_partitionnow differs from..._unpadded,experts/rocm_aiter_moe.pycomputes a non-zerointermediate_padwhere it previously got 0. Traced: withQuantType.No+ bf16 the dispatch falls past every cktile/flydsl branch (all requireq_type == per_1x32) to the plainck_moe_stage1/ck_moe_stage2_fwdpartials, which take no pad argument, andintermediate_padonly enters the tuned-config key whenconfig_file is not None, which vLLM never sets. So it is computed and discarded, and the kernel runs the full padded GEMM including the zero rows — up to ~14% extra compute on the MoE GEMMs (448->512 at TP=4, 224->256 at TP=8).2. AITER
biased_grouped_topkexpert-group count_aiter_get_num_expert_groupceil-dividesnum_expertsby the 32-per-group limit then walks to the next divisor, which can produce a count with no compiled kernel: AITER instantiatesbiased_grouped_topkonly forNUM_GRPin{1,2,4,8}, so 96 experts gives 3 and fails to launch.Grouping is a no-op here (
topk_group == num_expert_group), so any divisor within the limit routes identically — the constraint is purely which kernel exists. Round to the largest supported count that dividesnum_expertsand respects the limit. When none fits (320 -> 10, 33 -> 3) the naive value is kept deliberately: it is large enough that thetopk >= num_expert_groupguard at the call site fails and routing falls back to the generic path rather than reaching AITER.Test Plan
Both files are CPU-only and gated on
current_platform.is_rocm(), so they run in any ROCm CI job without a working AITER runtime.tests/kernels/moe/test_rocm_aiter_moe.py— new "Weight alignment" section appended to the existing file rather than a standalone module, following that file's conventions (function-local imports,default_vllm_config,rocm_naming):test_aiter_moe_alignment_follows_threshold— the rule, as a table across the boundary.test_aiter_moe_padded_size_stays_in_its_dispatch_branch— self-consistency:(intermediate <= 192) == (padded <= 192)andalignment(padded) == alignment(intermediate).test_aiter_moe_roundup_pads_intermediate— the hook end-to-end: 96->128, 192->192, 224->256, 448->512, 512->512.test_aiter_moe_roundup_is_not_applied_to_other_backends— TRITON and FLASHINFER_CUTLASS keep 224.test_aiter_moe_padded_weights_are_zero_initialized— pad lanes are zero, not allocator garbage.test_aiter_moe_aligned_weights_are_zero_initialized— zero-init is unconditional, so it holds for an aligned size too.test_aiter_moe_bias_matches_padded_weight—w13_bias.shape[1] == w13_weight.shape[1]withhas_bias=True; the defect the post-load approach had.test_aiter_moe_padding_is_numerically_inert— reproduces the loader's gate/up placement into a zeroed padded parameter, asserts the MoE output is bit-identical to the unpadded reference.tests/kernels/moe/test_rocm_aiter_num_expert_group.py(new) — five tests over_aiter_get_num_expert_group: the two invariants the router asserts unconditionally, that a supported count is chosen whenever one fits, a table of known expert counts, and that the fallback is taken deliberately when none fits.Test Result
gfx950(MI355X), torch2.11.0+gitd0c8b1f, on29af8bd672.The one failure,
test_aiter_fused_moe_mi3xx_fp8_accuracy, is pre-existing and unrelated — verified by running it alone against an unmodified checkout of the parent commit in a separate worktree, same box and image, where it fails identically.