Skip to content

[BugFix][Quantization] Surface FP8 kernel rejection reasons for fp8_per_channel - #61324

Open
YidaWeng wants to merge 1 commit into
vllm-project:mainfrom
YidaWeng:fix/fp8-kernel-rejection-reasons-61316
Open

YidaWeng wants to merge 1 commit into
vllm-project:mainfrom
YidaWeng:fix/fp8-kernel-rejection-reasons-61316

Conversation

@YidaWeng

@YidaWeng YidaWeng commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Make quantization="fp8_per_channel" fail with the scaled-mm chooser's per-kernel rejection reasons instead of a post-allocation Marlin/Humming guard. Fixes #61316.

Claims

  • MarlinFP8ScaledMMLinearKernel.can_implement and HummingFP8ScaledMMLinearKernel.can_implement reject per-token activation FP8 (W8A16 weight-only) with an explicit reason.
  • Both kernels still accept per-tensor activation configs used by the W8A16 / online per-tensor fallback path.
  • When no earlier FP8 kernel can implement a PTPC config, choose_scaled_mm_linear_kernel raises with the full Reasons: list, including Marlin and Humming rejections (no silent W8A16 selection).
  • Fp8PtpcOnlineLinearMethod.create_weights selects the kernel before allocating weights and prefixes chooser failures with quantization="fp8_per_channel".

Validation

.venv/bin/python -m pytest \
  tests/kernels/quantization/test_scaled_mm_kernel_selection.py \
  tests/quantization/test_fp8_per_channel.py \
  -v
# 30 passed, 3 skipped (GPU-only / gfx942-only)
uvx ruff check \
  vllm/model_executor/kernels/linear/scaled_mm/marlin.py \
  vllm/model_executor/kernels/linear/scaled_mm/humming.py \
  vllm/model_executor/layers/quantization/online/fp8.py \
  tests/kernels/quantization/test_scaled_mm_kernel_selection.py \
  tests/quantization/test_fp8_per_channel.py
# All checks passed

No GPU E2E against the issue reproducer (stablelm-3b-4e1t + fp8_per_channel) in this environment; the unit tests cover the chooser rejection path and the PTPC create_weights error wrapping.

Details

Root cause: Marlin's (and Humming's) can_implement returned (True, None) for every config. For PTPC (kFp8DynamicTokenSym + kFp8StaticChannelSym), earlier kernels may be rejected (no Cutlass FP8, no torch FP8 scaled_mm, etc.), a W8A16 kernel is selected, and the collected failure_reason_list is discarded. A later isinstance guard in Fp8PtpcOnlineLinearMethod.create_weights then raised a generic "Requires SM89+ / ROCm MI3xx" message after super().create_weights().

Approach: Reject per-token activation in Marlin and Humming so the chooser either falls through to a real W8A8 kernel (e.g. ChannelWiseTorch when available) or raises with all rejection reasons. Move PTPC kernel selection before weight allocation and wrap chooser ValueErrors with an fp8_per_channel prefix while preserving the Reasons list.

Follow-up from review: Humming sits after Marlin in _POSSIBLE_FP8_KERNELS. Without the same per-token rejection, Marlin's new refusal would fall through to Humming and reintroduce the silent-wrong-kernel failure. Also rebased onto main so the Marlin+Humming post-allocation guard from #60163 is replaced by the chooser path (and the now-unused Humming import in online/fp8.py is removed).

Tradeoff: Devices that previously selected Marlin/Humming for PTPC and then crashed will now either succeed via an earlier capable kernel or fail earlier with actionable Reasons. Per-tensor / W8A16 Marlin and Humming paths are unchanged.

Not duplicating: Searched open PRs for #61316, fp8 rejection reasons, and failure_reason_list. Related open work (#57600) gates Marlin on weight group shapes only; it does not reject per-token activation or fix the PTPC error path. #60163 already documented Humming as W8A16 for PTPC and added the post-allocation isinstance guard this PR replaces.

AI assistance was used to draft the change, tests, and this PR description. The human submitter reviewed every changed line and ran the tests above.

Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

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

@ShikhaPallavi123

Copy link
Copy Markdown

I ran this on a real GPU, since the description notes no GPU E2E was done.

Setup: RTX 4500 Ada (sm89, 24 GB), driver 590.48.01. I applied this PR's diff (git apply, applied cleanly) to a source build of main (torch 2.14.0+cu130, TORCH_CUDA_ARCH_LIST=8.9) and compared against the stock vllm 0.31.0 wheel (torch 2.13.0+cu130).

Issue repro (stabilityai/stablelm-3b-4e1t, quantization="fp8_per_channel", max_model_len=4096, gpu_memory_utilization=0.5, greedy, 8 tokens):

  • stock 0.31.0: Selected CutlassFP8ScaledMMLinearKernel for Fp8PtpcOnlineLinearMethod, output a city of contrasts.\nThe city
  • with this PR: same kernel selected, identical output

So on this card, where the CUTLASS FP8 kernel is available, I could not reproduce the crash from the issue, and this PR does not change the working path. I did not exercise the new error message end to end, since CUTLASS is supported here.

Tests: pytest tests/kernels/quantization/test_scaled_mm_kernel_selection.py tests/quantization/test_fp8_per_channel.py: 16 passed, 0 skipped on this GPU.

I used an AI assistant (Claude) to help set this up and summarize it. I ran the builds, tests and repro myself.

@mergify

mergify Bot commented Oct 27, 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, @YidaWeng.

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 Oct 27, 2026
@YidaWeng
YidaWeng force-pushed the fix/fp8-kernel-rejection-reasons-61316 branch from a81e4e6 to 063bae2 Compare October 28, 2026 15:16
@mergify mergify Bot removed the needs-rebase label Oct 28, 2026
@mergify

mergify Bot commented Oct 29, 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, @YidaWeng.

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 Oct 29, 2026
@selimfedakar

Copy link
Copy Markdown

Thanks for the fix — I hit the same error-reporting gap while triaging this area, so I read this closely. The approach (reject per-token in Marlin's can_implement + select the kernel before weight allocation) looks right, and I verified the main claims against current main. One issue I believe is blocking, plus a rebase note.

Blocking: HummingFP8ScaledMMLinearKernel reintroduces the exact bug this PR fixes.

In the CUDA chooser list (vllm/model_executor/kernels/linear/__init__.py, _POSSIBLE_FP8_KERNELS), Humming comes after Marlin:

FlashInfer → Cutlass → B12xTensor → PerTensorTorch → ChannelWiseTorch → Marlin → Humming

HummingFP8ScaledMMLinearKernel.can_implement unconditionally returns (True, None), and its is_supported only requires CUDA + humming installed + SM75+ (scaled_mm/humming.py). So on a pre-SM89 box with humming installed, this PR's new Marlin rejection makes the chooser fall through to Humming, which is silently selected for a PTPC config — a W8A16 weight-only kernel quietly serving a config that requested per-token activation FP8. That's the same silent-wrong-kernel failure mode as the original Marlin bug, just one slot later in the list.

This matters more because main has moved since this branch was cut: the post-allocation guard in Fp8PtpcOnlineLinearMethod.create_weights now excludes both Marlin and Humming (isinstance(self.fp8_linear, (MarlinFP8ScaledMMLinearKernel, HummingFP8ScaledMMLinearKernel))). This PR deletes the old Marlin-only version of that guard, so after a rebase the Humming protection is gone with nothing replacing it.

Suggested fix: add the same per-token rejection to HummingFP8ScaledMMLinearKernel.can_implement (it's W8A16 weight-only for the same reason as Marlin), plus a test mirroring test_marlin_fp8_rejects_per_token_activation. That also keeps the "either a real W8A8 kernel or a full Reasons list" invariant you state in the PR description.

Rebase note: besides the guard conflict above, once the tuple-guard is removed the HummingFP8ScaledMMLinearKernel import in online/fp8.py becomes unused → ruff will flag it.

Things I checked that look safe (sharing so reviewers don't have to re-derive them):

  • The Marlin can_implement change can't break the standard --quantization fp8 Marlin fallback on pre-SM89: Fp8LinearMethod only passes kFp8DynamicTokenSym when cutlass_fp8_supported() (or XPU) — otherwise it uses per-tensor, which Marlin still accepts (quantization/fp8.py, activation key selection in __init__).
  • compressed-tensors FP8_DYNAMIC checkpoints don't reach this path on old GPUs either — the scheme level already falls back to CompressedTensorsW8A16Fp8 below SM89 (compressed_tensors.py), so Marlin never sees a per-token key from there.
  • The pre-allocation weight_shape = (sum(output_partition_sizes), input_size_per_partition) matches what OnlineLinearBase.create_weights actually allocates ((output_size_per_partition, input_size_per_partition)), so moving kernel selection before super().create_weights() doesn't change the config the chooser sees. It's arguably more robust than the old layer.weight.shape read, which assumed the standard weight path.

AI assistance was used while verifying this review against main; I checked every referenced code path myself.

…er_channel

MarlinFP8ScaledMMLinearKernel and HummingFP8ScaledMMLinearKernel
can_implement accepted every config, so the scaled-mm chooser could return a
W8A16 weight-only kernel for PTPC (per-token activation) and discard earlier
kernel rejection reasons. The PTPC path then raised a generic SM89+/ROCm
message after weight allocation.

Reject per-token activation in both Marlin and Humming (W8A16 weight-only),
select the kernel before create_weights allocates, and wrap chooser failures
with an fp8_per_channel-prefixed message that keeps the per-kernel Reasons
list.

Fixes vllm-project#61316

Co-authored-by: Grok Build <grok@x.ai>
Signed-off-by: YidaWeng <84164537+YidaWeng@users.noreply.github.com>
@YidaWeng
YidaWeng force-pushed the fix/fp8-kernel-rejection-reasons-61316 branch from 063bae2 to a6c541a Compare October 31, 2026 09:15
@YidaWeng

Copy link
Copy Markdown
Contributor Author

Addressed the Humming follow-up from #61324 (comment):

  • Rebased onto current main (resolves the Marlin+Humming post-allocation guard conflict from [Bugfix][Quantization] Fix Humming FP8 linear for per-tensor/per-channel FP8 weights #60163).
  • HummingFP8ScaledMMLinearKernel.can_implement now rejects per-token activation with the same W8A16 reason as Marlin.
  • Added test_humming_fp8_rejects_per_token_activation / test_humming_fp8_accepts_per_tensor_activation, and extended the chooser test so both Marlin and Humming appear in the Reasons: list.
  • Removed the now-unused HummingFP8ScaledMMLinearKernel import from online/fp8.py.

Validation: 30 passed, 3 skipped on the kernel-selection + fp8_per_channel unit tests; ruff clean on the touched files.

@mergify mergify Bot removed the needs-rebase label Oct 31, 2026
@selimfedakar

Copy link
Copy Markdown

Thanks for the thorough turnaround — I checked the update and it closes the concern:

  • HummingFP8ScaledMMLinearKernel.can_implement now rejects per-token activation (config.activation_quant_key.scale.group_shape.is_per_token() → False with the W8A16 weight-only reason), so the chooser can no longer fall silently from Marlin's per-token rejection into Humming on pre-SM89 — that was the slot I was worried about.
  • Rebasing onto the (Marlin, Humming) post-allocation guard from [Bugfix][Quantization] Fix Humming FP8 linear for per-tensor/per-channel FP8 weights #60163 resolves the staleness, and dropping the now-unused HummingFP8ScaledMMLinearKernel import from online/fp8.py clears the ruff issue.
  • The new test_humming_fp8_rejects_per_token_activation / _accepts_per_tensor_activation plus the extended chooser test (both kernels in the Reasons: list) cover exactly the path that was silent before.

LGTM on my end (non-binding — I don't have write access).

(Verified against the updated branch and current main; prepared with AI assistance.)

@mergify

mergify Bot commented Nov 3, 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, @YidaWeng.

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 Nov 3, 2026

This branch has not been deployed

No deployments
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

3 participants