Skip to content

[ROCm][CI][AiterExperts] Add test coverage for hidden/intermediate padding correctness - #59333

Merged
AndreasKaratzas merged 6 commits into
vllm-project:mainfrom
divakar-amd:aiter-padding-4b
Oct 3, 2026
Merged

AndreasKaratzas merged 6 commits into
vllm-project:mainfrom
divakar-amd:aiter-padding-4b

Conversation

@divakar-amd

@divakar-amd divakar-amd commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Covers the logical hidden_dim_unpadded / intermediate_size_per_partition_unpadded portion of the "Test padding" (4b) ask in #54966. These FusedMoEConfig fields drive AiterExperts's hidden_pad/intermediate_pad computation so AITER's CK/FlyDSL kernels can strip padded regions internally; no existing test forces them to diverge from the padded size for any quant family other than MXFP4's own coverage (tests/quantization/test_online_mxfp4.py).

This PR:

  • Adds optional Config.hidden_dim_unpadded / Config.intermediate_size_per_partition_unpadded fields to the modular kernel test harness (tests/kernels/moe/modular_kernel_tools/common.py), defaulting to None (today's behavior, unchanged everywhere else).
  • Adds test_aiter_moe_padding_matrix, parametrized across {no padding, hidden-only, intermediate-only, both} x {unquantized, fp8_channel_token, fp8_tensor_tensor, fp8_tensor_token, fp8_block_token}. Asserts AiterExperts.apply() forwards the expected hidden_pad/intermediate_pad to rocm_aiter_ops.fused_moe, that its output buffer is still the raw (padded) width, then slices that buffer down to the real hidden_dim_unpadded region and compares it against reference_moe_impl run at the true unpadded size.
  • Adds a skipped MXFP4 stub pending MI350/gfx950 hardware, consistent with the existing stub pattern from earlier steps of this plan.

2 of the 20 test_aiter_moe_padding_matrix cases (block-quantized FP8 x {no padding, intermediate-only}) are runtime-skipped: AITER's block CK GEMM kernel rejects the raw unpadded hidden_dim shape on this hardware, so only the
hidden_pad-forcing modes are exercised for that quant scheme.

Test plan

VLLM_ROCM_USE_AITER=1 VLLM_ROCM_USE_AITER_MOE=1 pytest -v -s tests/kernels/moe/test_modular_kernel_combinations.py -k padding_matrix

Plus a full-file regression run of test_modular_kernel_combinations.py to confirm no regressions to existing AITER coverage.

AI assistance disclosure

This PR was developed with assistance from Claude Code and reviewed by me.

Covers step 4b of the "Test padding" ask in vllm-project#54966:
adds optional Config.hidden_dim_unpadded /
intermediate_size_per_partition_unpadded fields (default None,
preserving today's behavior everywhere else) threaded through to
FusedMoEConfig in make_modular_kernel(), and a parametrized
test_aiter_moe_padding_matrix covering the {no padding, hidden-only,
intermediate-only, both} x {unquantized, fp8_channel_token,
fp8_tensor_tensor, fp8_tensor_token, fp8_block_token} matrix for
AiterExperts, using deliberately unaligned K/N so hidden_pad/
intermediate_pad actually diverge from zero. Compares against
reference_moe_impl run at the true unpadded size and asserts the
modular kernel's output is already unpadded-width (AiterExperts.apply()
never slices its own output). Adds a skipped MXFP4 stub pending
MI350/gfx950 hardware.

Gated via require_aiter_moe / has_aiter(); inert when AITER isn't
available or off ROCm.

Signed-off-by: Divakar Verma <divakar.verma@amd.com>
Check torch.isfinite() only on AiterExperts.apply()'s real
hidden_dim_unpadded-wide output slice, not the full padded-width
buffer: the tail beyond hidden_dim_unpadded is unspecified/reused
memory, not something AiterExperts promises to leave finite, so
asserting on it risked a spurious failure unrelated to any real bug.

Drop the redundant @pytest.mark.parametrize and inner pytest.skip()
on test_aiter_moe_padding_matrix_mxfp4: the outer @pytest.mark.skip
already prevents the body from ever running, so both were dead
weight collecting 4 identical skipped test IDs instead of 1.

Signed-off-by: Divakar Verma <divakar.verma@amd.com>
Signed-off-by: Divakar Verma <divakar.verma@amd.com>
@divakar-amd divakar-amd changed the title [ROCm] Test AiterExperts logical hidden/intermediate padding matrix [ROCm][CI][AiterExperts] Add test coverage for hidden/intermediate padding correctness Oct 2, 2026
@divakar-amd

Copy link
Copy Markdown
Contributor Author

/ci run

@divakar-amd
divakar-amd marked this pull request as ready for review October 2, 2026 21:32
Copilot AI balanced review requested due to automatic review settings October 2, 2026 21:32

@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 pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92709 for commit 7c698ed20b16.

Comment thread tests/kernels/moe/test_modular_kernel_combinations.py
Comment on lines +696 to +709
# K/N intentionally unaligned to AITER's 64/128 granularity (unlike the
# shared mk_objects.py defaults) so hidden_pad/intermediate_pad are forced.
_PADDING_E = 8
_PADDING_M = 16
_PADDING_TOPK = 2
_PADDING_K_UNPADDED = 896 # 7 * 128
_PADDING_K_PADDED = 1024 # +128
_PADDING_N_UNPADDED = 384 # 3 * 128
_PADDING_N_PADDED = 512 # +128
# hidden_pad = (K_padded - hidden_dim_unpadded) // 128 * 128 (rocm_aiter_moe.py)
_PADDING_HIDDEN_PAD_EXPECTED = 128
# intermediate_pad = (N_padded - intermediate_size_per_partition_unpadded)
# // 64 * 64 * (2 if tp_size == 1 else 1) (rocm_aiter_moe.py); tp_size==1 here.
_PADDING_INTERMEDIATE_PAD_EXPECTED = 256

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide brief explanation of these hyper params and why these over a grid set ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the usual model params like number of Experts, num tokens, hidden_dim etc. This is for a single fixed small example model size rather than a grid set.
The only grid in this PR is the 4 (padding mode) × 5 (quant scheme) = 20 case-matrix

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Numerical comparisons can miss padding errors, and the tensor-token FP8 cases use incompatible weight scales.

Review effort: Balanced
Findings: 3 Medium severity

Open (3)
What changed in this PR

Adds ROCm AiterExperts padding coverage to the modular MoE test harness, addressing part of #54966.

Changes:

  • Adds optional unpadded dimensions to the shared test configuration.
  • Tests padding argument forwarding, output width, and numerical correctness across unquantized and FP8 configurations.
  • Adds an MXFP4 placeholder and skips two unsupported block-FP8 shapes.
File Description
tests/​kernels/​moe/​test_modular_kernel_combinations.py Adds padding scenarios, reference slicing helpers, and assertions.
tests/​kernels/​moe/​modular_kernel_tools/​common.py Passes optional unpadded dimensions into FusedMoEConfig.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/kernels/moe/test_modular_kernel_combinations.py Outdated
Comment thread tests/kernels/moe/test_modular_kernel_combinations.py Outdated
Comment thread tests/kernels/moe/test_modular_kernel_combinations.py Outdated
@vllm-agent

vllm-agent commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

CI selector (shadow): 1 test steps (5 jobs) instead of 6 (10 jobs)

Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works.

Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.

steps (jobs) Today's rules Selector Would skip Would add
NVIDIA, CPU and others 6 (10) 1 (5) 5 (5) 0 (0)
AMD mirrors 7 (11) 1 (5) 6 (6) 0 (0)
Selector would run (1)
  • kernels-moe-test ×5
Would skip (today's rules run them) (5)
  • kernels-fla-ops-test-b200
  • kernels-fusedmoe-layer-test-2-b200s
  • kernels-fusedmoe-layer-test-2-h100s
  • kernels-mhc-test-b200
  • kernels-root-misc-test-b200
Would add (today's rules do not run them) (0)

none

AMD mirrors: would skip (6)
  • fault-tolerance-e2e-2xh100
  • kernels-fla-ops-test-b200
  • kernels-fusedmoe-layer-test-2-b200s
  • kernels-fusedmoe-layer-test-2-h100s
  • kernels-mhc-test-b200
  • kernels-root-misc-test-b200
AMD mirrors: would add (0)

none

2 changed files · base 44198f577f · head c3238b2460 · Python record: build 92706 at 6e517b15c1 · kernel record: table 6e517b1 (build 92706), map 6e517b1 · not counted: 10 build steps, 5 A100 steps the generator no longer emits, 0 optional steps the selector would also run

- Always use the lenient check_accuracy path for the padding-matrix
  comparison, instead of a strict/lenient branch whose condition was
  always true (dead code).
- Add a magnitude-ratio guard before check_accuracy: elementwise
  atol/rtol alone wouldn't catch a degenerate/zeroed output here, since
  ref_out's magnitude is below atol at this problem size.
- Add _make_padding_matrix_weights(), scoped to this test, so the
  fp8_tensor_token case gets per-tensor (not per-channel) weight
  scales, matching the quant scheme it declares.

Signed-off-by: Divakar Verma <divakar.verma@amd.com>
@AndreasKaratzas

Copy link
Copy Markdown
Member

/ci run

@AndreasKaratzas
AndreasKaratzas enabled auto-merge (squash) October 3, 2026 01:05
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 3, 2026
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92725 for commit c3238b2460a3.

@AndreasKaratzas
AndreasKaratzas merged commit faa9860 into vllm-project:main Oct 3, 2026
42 checks passed
divakar-amd added a commit to divakar-amd/vllm_upstream that referenced this pull request Oct 8, 2026
ref_out's typical magnitude here (~1e-3-1e-2) sits below the 3e-2 atol
used by both check_accuracy and torch.testing.assert_close, so a fully
zeroed-out mk_out -- e.g. a garbage row's inf/nan corrupting the real
rows via some leak the isfinite check doesn't catch (zero isn't
NaN/Inf) -- would still spuriously pass. Mirrors the same guard added
to the stacked 4b padding-matrix test (vllm-project#59333) for the same reason.

Signed-off-by: Divakar Verma <divakar.verma@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants