Repository navigation
[ROCm][CI] Add coverage for AiterExperts token padding: inf/nan garbage rows - #59334
Merged
Merged
Conversation
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Covers step 4c of the "Test padding" ask in vllm-project#54966: simulates the HIP-graph padding-token contract (grouped_topk_router.py masking padding rows to topk_ids=-1 / topk_weights=0) at the tensor level, without needing real HIP-graph capture. Fills a subset of rows' hidden_states with inf/-inf/nan and masks their routing accordingly, then verifies through AiterExperts that: (1) no NaN/Inf leaks into the real (non-garbage) rows' output, and (2) the real rows' output matches reference_moe_impl computed only over those rows. Parametrized across 3 garbage-row patterns (single inf, single nan, multiple mixed) x 5 quant schemes AiterExperts supports (unquantized + 4 fp8 variants). Adds a skipped MXFP4 stub pending MI350/gfx950 hardware. Builds on the Config.hidden_dim_unpadded / intermediate_size_per_partition_unpadded plumbing and _PADDING_* constants added in the 4b PR (stacked on that branch since this reuses those constants directly). 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>
The outer @pytest.mark.skip already prevents this test from ever running; the inner pytest.skip() call is unreachable dead code. Signed-off-by: Divakar Verma <divakar.verma@amd.com>
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>
_TOKEN_PADDING_M/K/N duplicated _PADDING_M/_PADDING_K_PADDED/ _PADDING_N_PADDED's values under new names. Reusing the originals avoids the two sets silently drifting out of sync. Signed-off-by: Divakar Verma <divakar.verma@amd.com>
divakar-amd
force-pushed
the
aiter-padding-4c
branch
from
October 8, 2026 15:11
d807c0b to
ed20edd
Compare
AiterExperts falls back to AITER's dynamic per-tensor activation quant for this scheme (rocm_aiter_moe.py ignores per_act_token_quant when weights are per-tensor), whose scale is a max-abs reduction over the whole batch. A real inf garbage row legitimately poisons that shared scale, zeroing every real row's output -- not something AiterExperts can be expected to isolate against today. Filed upstream as ROCm/aiter#6275. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Divakar Verma <divakar.verma@amd.com>
Avoids re-deriving (per_out_ch_quant, per_act_token_quant, block_shape) by hand -- matches the existing assert_aiter_quant_scheme_case() pattern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Divakar Verma <divakar.verma@amd.com>
divakar-amd
changed the title
The dropped lines duplicated what the test's own docstring already says (what the HIP-graph padding contract simulates); keep only the non-duplicated "why this is a valid comparison" rationale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Divakar Verma <divakar.verma@amd.com>
divakar-amd
marked this pull request as ready for review
October 8, 2026 18:11
divakar-amd
requested review from
AndreasKaratzas,
WoosukKwon,
mgoin,
tlrmchlsmth,
yewentao256 and
zyongye
as code owners
October 8, 2026 18:11
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Member
|
/ci run |
AndreasKaratzas
approved these changes
Oct 8, 2026
AndreasKaratzas
enabled auto-merge (squash)
October 8, 2026 18:20
github-actions
Bot
added
the
ready
|
✅ Triggered Buildkite CI #93671 for commit |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The unquantized assertion branch is unreachable, and one quantized case uses the wrong weight-scale layout.
2 open findings
What changed in this PR
Adds ROCm CI coverage for AiterExperts handling of non-finite HIP-graph padding rows.
Changes:
- Tests three Inf/NaN padding patterns across five quantization schemes.
- Validates finite, non-degenerate real-row outputs against the reference implementation.
- Adds a skipped MXFP4 placeholder.
| File | Description |
|---|---|
tests/kernels/moe/test_modular_kernel_combinations.py |
Adds token-padding test helpers, matrix coverage, and MXFP4 stub. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| f"degenerate/zeroed vs. reference (mean |ref_out|={ref_scale:.6f})." | ||
| ) | ||
|
|
||
| if config.quant_config is not None: |
| "https://github.com/ROCm/aiter/issues/6275." | ||
| ) | ||
|
|
||
| weights = WeightTensors.make(config) |
1 task done
michael-gpu
pushed a commit
to practical-inference/vllm-ampere-extended
that referenced
this pull request
Oct 20, 2026
…ge rows (vllm-project#59334) Signed-off-by: Divakar Verma <divakar.verma@amd.com> Co-authored-by: Andreas Karatzas <akaratza@amd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Covers the HIP-graph token-padding portion of the "Test padding" ask in #54966: garbage/padding token rows in a captured graph bucket can carry inf/nan
hidden_states, masked via the router'stopk_ids=-1/topk_weights=0contract (grouped_topk_router.py). This is a distinct mechanism from #53108 (which fixesAiterExpertszeroing valid rows when its input buffer is oversized relative to the real token count, an EP-dispatch buffer-sizing issue) -- no existing test provesAiterExpertshandles the HIP-graph capture-bucket inf/nan case safely.Adds
test_aiter_moe_token_padding_garbage_rows, which simulates this contract at the tensor level (no real HIP-graph capture needed): fills a subset of rows with inf/-inf/nan and masks their routing, then verifies throughAiterExpertsthat (1) no NaN/Inf leaks into the real rows' output, and (2) real rows' output matchesreference_moe_implcomputed only over those rows. Parametrized across 3 garbage-row patterns (single inf, single nan, multiple mixed) x 5 quant schemes iterExperts supports. Adds a skipped MXFP4 stub pending MI350/gfx950 hardware.Stacked on #59333 (4b), since this reuses the
_PADDING_*constants andConfig.hidden_dim_unpaddedplumbing added there. GitHub doesn't support a fork-only branch as a PR base against upstream, so this PR's base ismainfor now -- the diff currently includes both #59333's commit and this PR's own commit. Only the top commit (token padding: inf/nan garbage rows) needs review here; #59333's commit is under review in that PR. Once #59333 merges, I'll rebase this branch onto the updatedmainso the diff collapses to just this PR's change.Test plan
Plus a full-file regression run to confirm no regressions.
AI assistance disclosure
This PR was developed with assistance from Claude Code (Anthropic), including test design, implementation, and verification. All test results were independently run and reviewed by me.