Skip to content

[Bugfix] Support non-gated MoE in online quantization and Marlin MoE tile padding - #48028

Closed
puririshi98 wants to merge 10 commits into
vllm-project:mainfrom
puririshi98:nongated-online-quant-marlin-pad
Closed

puririshi98 wants to merge 10 commits into
vllm-project:mainfrom
puririshi98:nongated-online-quant-marlin-pad

Conversation

@puririshi98

@puririshi98 puririshi98 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Purpose

Non-gated MoE models — where w13 holds only an up projection instead of fused
gate+up shards — currently break in two places on the online-quantization +
Marlin path. The concrete motivation is the NemotronH architecture
(mlp_hidden_act="relu2", is_act_and_mul=False) used by the public
Nemotron-3 family:

MarlinExperts already supports non-gated MoE (_supports_no_act_and_mul() -> True, RELU2_NO_MUL, w13_num_shards = 2 if activation.is_gated else 1), but
the weight-prep side does not:

Bug 1 — online-quant allocation.
OnlineMoEMethodBase.create_weights unconditionally allocates w13_weight
(and w13_bias) at 2 * intermediate_size_per_partition. For non-gated
models the kernel-format conversion then packs a 2N-wide w13 while the
Marlin MoE gemm computes size_n = N, tripping
STD_TORCH_CHECK(size_n == actual_size_n) in marlin_moe_wna16. This is the
shared allocation base inherited by all online MoE methods (fp8
per-tensor/per-block/ptpc, mxfp8, int8), so one fix covers all of them; it
mirrors UnquantizedFusedMoEMethod.create_weights (w13_up_dim). The fix also
makes Fp8PerBlockOnlineMoEMethod._zero_padding shard-aware: it
previously assumed two gate/up shards, so for a non-gated model whose
intermediate size is rounded up to the 128 quant block, uninitialized pad rows
were never zeroed and would contaminate the shared per-block scales.

Bug 2 — Marlin MoE tile padding is gated-only.
The MoE thread-tile padding added in #45703 hardcodes the gated layout:
_moe_pad_shard_rows views (E, 2, n, ...) and the fp8 scale-pad branch
views (e, g, 2, n). A non-gated model with a tile-misaligned rank-local
intermediate size fails at the view instead of being padded. Concrete case:
Nemotron-3-Super-120B-A12B has moe_intermediate_size = 2688; at TP=4 the
rank-local shard is 672, which is not a multiple of the 64-column Marlin
thread tile (min_thread_n). The fix derives the shard count from the tensor
shapes (w13_n // n: 2 gated, 1 non-gated) and threads it through the weight
and scale padding in prepare_fp8_moe_layer_for_marlin and
prepare_mxfp8_moe_layer_for_marlin. Zero-padding is mathematically exact for
non-gated activations with act(0) = 0 (relu²): the padded w13 output
channels are zero, and the matching zero w2 input columns drop out.

No behavior change for existing gated models: num_w13_shards resolves
to 2 and reproduces the prior layout exactly; the allocation and zero-padding
changes only affect is_act_and_mul=False configurations, which previously
crashed (or, for per-block zero-padding, silently corrupted scales).

Why this does not duplicate existing PRs

Changes

  1. vllm/model_executor/layers/quantization/online/moe_base.py — size
    w13_weight/w13_bias by self.moe.is_act_and_mul.
  2. vllm/model_executor/layers/quantization/online/fp8.py — shard-aware
    Fp8PerBlockOnlineMoEMethod._zero_padding.
  3. vllm/model_executor/layers/quantization/utils/marlin_utils_fp8.py
    _moe_pad_shard_rows(..., num_shards) (required parameter);
    shard-count-aware weight and scale padding in both MoE prep functions.
  4. Tests:
    • tests/quantization/test_online.py — CPU unit tests
      test_online_moe_create_weights_w13_dim (gated × bias) and
      test_fp8_per_block_zero_padding (gated/non-gated).
    • tests/kernels/quantization/test_marlin_tile_padding.py — CPU shape test
      test_moe_pad_shard_rows[1|2]; gated=[True, False] parametrization of
      test_fp8_marlin_moe_padded_round_trip and
      test_mxfp8_marlin_moe_padded_round_trip (non-gated uses
      MoEActivation.RELU2_NO_MUL in both the kernel call and the
      torch_experts reference); round-trip inputs are no longer downscaled so
      the tolerance is discriminative, and the round-trips are seeded
      (torch.manual_seed(0)) so the unscaled inputs stay deterministic across
      runs.

Known related issue left out of scope: the int-WNA16 oracle pad helpers
(_pad_w13_shard_cols/_pad_w13_bias in fused_moe/oracle/int_wna16.py)
are still gated-only, so a non-gated WNA16 model with a tile-misaligned
intermediate fails loudly at the analogous view; same fix pattern applies
and can follow up.

Test Plan

# GPU: end-to-end online-quant MoE (weight creation -> loading -> post-load
# processing -> kernel run), gated and non-gated
pytest tests/quantization/test_online.py -v -k end_to_end

# GPU: full Marlin tile-padding regression (fp8-Marlin-capable GPU, sm >= 80)
pytest tests/kernels/quantization/test_marlin_tile_padding.py -v

Test Result

All runs on GB200 (aarch64, CUDA 13.0), editable install of this branch with
the precompiled wheel pinned to the main commit this branch merges
(1ff942965), so the compiled ops match the source.

$ pytest tests/quantization/test_online.py -q -k end_to_end
2 passed, 6 deselected in 25.63s

$ pytest tests/kernels/quantization/test_marlin_tile_padding.py -q
103 passed, 13 skipped in 30.42s

# canary: a Marlin MoE test this PR does not touch
$ pytest tests/kernels/quantization/test_marlin_tile_padding.py -q \
    -k test_gptq_marlin_moe_padded_round_trip
4 passed in 45.71s

Negative controls. The end-to-end test was confirmed to fail when either
fix is reverted, and only for the non-gated parametrization:

Reverted fix Result
create_weights w13 allocation back to unconditional 2 * intermediate [False] fails, [True] passes
_zero_padding back to assuming 2 shards [False] fails, [True] passes

The test uses a non-block-aligned intermediate size (n=96, rounded up to the
128 quant block) and leaves the roundup pad rows non-zero, so the kernel output
only matches the bf16 reference if the pad rows are zeroed — this is what makes
the second control fail rather than silently pass.

The test pins VLLM_TEST_FORCE_FP8_MARLIN=1. Marlin is the fp8 MoE backend
that supports non-gated MoE; the Blackwell default
trtllm_fp8_block_scale_moe hardcodes the gated layout and rejects a
single-shard w13 (gemm1_weights_scale.size(1) == intermediate_size_factor * intermediate_size / 128, 1 vs 2). That is a separate gap, not addressed here.

The 13 skips are pre-existing shape/group-divisibility guards in
test_gptq_marlin_padded_round_trip / test_marlin_moe_padded_intermediate
("group straddles the boundary", "group must divide the rank-local K") and
skip identically on upstream main; none are in tests added or modified by
this PR.

Additional validation: a functionally equivalent backport of both fixes to
v0.20.0 was validated on GB200:

  • End-to-end online-MXFP8 + Marlin generation for all three Nemotron-3 models
    (Nano-30B-A3B, Super-120B-A12B at TP=4 exercising the 672->704 pad,
    Ultra-550B-A55B at TP=8/PP=2).
  • GEMM-level parity for the padded non-gated case: fused Marlin MoE output vs
    a bf16 reference reaches the same agreement as an already-tile-aligned
    control (cosine similarity 0.999180 padded vs 0.999174 control, i.e. the
    padding contributes no error beyond MXFP8 quantization noise). Since the
    change only enables previously-crashing configurations and gated layouts are
    byte-identical, this parity evidence stands in for a model-quality eval.

AI assistance disclosure

This PR was developed with AI assistance (Claude Code), including an
adversarial multi-agent review pass. Every changed line has been reviewed by
the human submitter, who ran the tests and can defend the change end-to-end.

@mergify mergify Bot added the bug Something isn't working label Jul 8, 2026
@puririshi98
puririshi98 marked this pull request as ready for review July 8, 2026 20:49

@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.

puririshi98 and others added 2 commits July 11, 2026 17:09
OnlineMoEMethodBase.create_weights unconditionally allocates w13_weight
(and w13_bias) at 2 * intermediate_size_per_partition, assuming a fused
gate_up_proj. Non-gated MoE models (is_act_and_mul=False, e.g. NemotronH
relu2 in Nemotron-3 Nano/Super/Ultra) have no gate projection, so the
kernel-format conversion packs a 2N-wide w13 while the Marlin MoE gemm
computes size_n = N (w13_num_shards = 1), tripping
'size_n == actual_size_n' in marlin_moe_wna16.

Size w13 by is_act_and_mul, matching UnquantizedFusedMoEMethod. This is
the shared allocation base inherited by all online MoE methods (fp8
per-tensor/per-block/ptpc, mxfp8, int8). Also make
Fp8PerBlockOnlineMoEMethod._zero_padding shard-aware so the roundup
padding of a non-gated w13 is zeroed correctly (previously it assumed
two gate/up shards and would leave uninitialized pad rows that
contaminate the shared per-block scales).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Rishi Puri <riship@nvidia.com>
… MoE

The Marlin MoE thread-tile padding (vllm-project#45703) hardcodes the gated w13
layout: _moe_pad_shard_rows views (E, 2, n, ...) and the fp8 scale pad
branch views (e, g, 2, n). For non-gated MoE (is_act_and_mul=False) w13
is (E, N, K), so a tile-misaligned rank-local intermediate size (e.g.
Nemotron-3-Super-120B-A12B: moe_intermediate_size 2688 at TP4 -> 672,
not a multiple of the 64-column Marlin thread tile) fails at the view
instead of being padded, even though MarlinExperts supports
RELU2_NO_MUL.

Derive the w13 shard count from the tensor shapes (w13_n // n: 2 gated,
1 non-gated) and thread it through the weight and scale padding in
prepare_fp8_moe_layer_for_marlin and prepare_mxfp8_moe_layer_for_marlin
(num_shards is a required parameter to keep future callers explicit).
Extend the padded round-trip tests to cover both layouts and use
unscaled inputs so the tolerance is discriminative.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Rishi Puri <riship@nvidia.com>
@puririshi98
puririshi98 force-pushed the nongated-online-quant-marlin-pad branch from bf7586f to 8eca7df Compare July 12, 2026 00:09
puririshi98 and others added 2 commits July 11, 2026 19:55
Seed the fp8/mxfp8 Marlin MoE padded round-trip tests
(torch.manual_seed(0)) so the unscaled random inputs are deterministic
against the fixed atol=8e-2 tolerance, removing a latent flakiness risk
introduced when the /10 input downscale was dropped.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Rishi Puri <riship@nvidia.com>
@puririshi98

Copy link
Copy Markdown
Contributor Author

@TomerBN-Nvidia do you know who can merge this?

@TomerBN-Nvidia

Copy link
Copy Markdown
Contributor

@TomerBN-Nvidia do you know who can merge this?

@pavanimajety can you help with this?

@pavanimajety pavanimajety left a comment

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.

A few nits, thanks for the PR!

Comment thread tests/quantization/test_online.py Outdated
Comment on lines +181 to +185
@pytest.mark.parametrize("is_act_and_mul", [True, False])
@pytest.mark.parametrize("has_bias", [True, False])
def test_online_moe_create_weights_w13_dim(is_act_and_mul, has_bias):
"""w13 holds gate+up (2N) for gated MoE and up only (N) for non-gated MoE
(is_act_and_mul=False, e.g. NemotronH relu2)."""

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.

nit: I think this test is unnecessary. it might be more helpful to add one test that tests the whole online_moe weight creation, post weight loading and kernel run.

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.

Agreed, replaced both narrow tests with a single end-to-end test_online_moe_fp8_per_block_end_to_end that covers weight creation, weight loading, post-loading processing and a kernel run (parametrized gated / non-gated).

Two details worth calling out:

  • It uses a non-block-aligned intermediate size (n=96, rounded up to 128) and leaves the roundup pad rows non-zero, so the kernel output only matches the bf16 reference if _zero_padding actually zeroes them — that turns the old shape/zero-padding assertions into a behavioural guard.
  • It pins VLLM_TEST_FORCE_FP8_MARLIN=1. Marlin is the fp8 MoE backend that supports non-gated MoE; on Blackwell the default trtllm_fp8_block_scale_moe hardcodes the gated layout and rejects a single-shard w13 (gemm1_weights_scale.size(1) == intermediate_size_factor * intermediate_size / 128 (1 vs 2)), which is a separate gap outside this PR.

Verified on GB200: 2 passed. Negative controls confirm it catches both bugs — reverting the w13 allocation fix, or making _zero_padding assume 2 shards again, fails only the non-gated case.

Comment on lines +602 to +606
w13_bias_shard_size = layer.w13_bias.shape[1] // num_w13_shards
if w13_bias_shard_size > intermediate_size:
for s in range(num_w13_shards):
start = s * w13_bias_shard_size + intermediate_size
layer.w13_bias[:, start : (s + 1) * w13_bias_shard_size] = 0

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.

nit: Combine with the above loop

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.

Done — the weight and bias pad-zeroing now share one loop:

num_w13_shards = 2 if self.moe.is_act_and_mul else 1
for w13 in (layer.w13_weight, getattr(layer, "w13_bias", None)):
    if w13 is None:
        continue
    shard_size = w13.shape[1] // num_w13_shards
    if shard_size > intermediate_size:
        for s in range(num_w13_shards):
            start = s * shard_size + intermediate_size
            w13[:, start : (s + 1) * shard_size] = 0

Semantics are unchanged: for the 3-D weight, w13[:, a:b] is equivalent to the previous w13[:, a:b, :].

Comment on lines +220 to +222
def _moe_pad_shard_rows(
x: torch.Tensor, n: int, padded_n: int, num_shards: int
) -> torch.Tensor:

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.

are there other places where _moe_pad_shard_rows is utilized? Do we need to give a default number here for num_shards?

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.

_moe_pad_shard_rows is module-private and has exactly three call sites, all in marlin_utils_fp8.py, and all now pass num_w13_shards explicitly:

  • prepare_fp8_moe_layer_for_marlinw13_weight
  • prepare_mxfp8_moe_layer_for_marlinw13 and w13_scale

I would prefer not to give num_shards a default. A default of 2 would silently reinstate the gated-only assumption this PR is fixing: a non-gated layer would then be mis-padded rather than failing loudly at the view. Keeping it required means any future caller has to state the layout. Happy to add one if you feel strongly.

@mergify mergify Bot added the quantization label Jul 23, 2026
puririshi98 and others added 2 commits July 27, 2026 20:14
Per review feedback:
- Replace the two narrow online-MoE unit tests with a single end-to-end test
  covering weight creation, weight loading, post-loading processing and a
  kernel run, for gated and non-gated layouts. It uses a non-block-aligned
  intermediate size and leaves the roundup pad rows non-zero, so the result
  only matches the reference if the pad rows are zeroed. Pinned to the Marlin
  MoE backend, the fp8 backend that supports non-gated MoE.
- Combine the w13 weight and bias pad-zeroing loops in _zero_padding.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Rishi Puri <riship@nvidia.com>
puririshi98 and others added 2 commits July 28, 2026 15:50
- ruff-format the online MoE test (the pre-commit CI failure).
- Use the dist_init fixture instead of a hand-rolled distributed init: it
  uses a file:// store rather than a fixed TCP port and tears the
  distributed environment down after the test.
- Make the numeric comparison meaningful: the previous tolerance was larger
  than the signal, so a zeroed output would have passed. Tighten it and
  assert the reference is non-trivial.
- Assert directly that the w13 pad rows are zeroed. w2's pad columns are
  zeroed unconditionally, so a non-zeroed w13 pad row cannot be observed in
  the kernel output, but it does contaminate the shared per-block scale.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Rishi Puri <riship@nvidia.com>
@aoshen02

aoshen02 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Hi, thanks for the pr. I've solve the problem partially in #51125 and list you as an author. Hope it works for you, lmk if any other problems.

@mergify

mergify Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @puririshi98.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Aug 6, 2026
@puririshi98 puririshi98 closed this Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working needs-rebase quantization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants