Skip to content

[Online quantization] Support activation quant key override - #51808

Open
fxmarty-amd wants to merge 1 commit into
vllm-project:mainfrom
fxmarty-amd:relax-online-quant-act-key-override
Open

[Online quantization] Support activation quant key override#51808
fxmarty-amd wants to merge 1 commit into
vllm-project:mainfrom
fxmarty-amd:relax-online-quant-act-key-override

Conversation

@fxmarty-amd

@fxmarty-amd fxmarty-amd commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Purpose

Support --quantization online --quantization-config.moe.activation xyz API.

Currently, activation override for online quantization raises a NotImplementedError, but overriding the activation quant key is truly needed to dispatch on preferred MOE backends (e.g. AITER_MXFP4_BF16, AITER_MXFP4_MXFP4, AITER_MXFP4_FP8) that can not be selected in any other way.

More context at #52167

Test Plan

  • pytest tests/quantization/test_quantization_config_args.py -s -vvvvv
  • pytest tests/quantization/test_online.py -s -vvvvv

Test Result

Passing on MI350

@mergify mergify Bot added the quantization label Aug 11, 2026
@mergify

mergify Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--51808.org.readthedocs.build/en/51808/

@mergify mergify Bot added the documentation Improvements or additions to documentation label Aug 11, 2026
@fxmarty-amd
fxmarty-amd marked this pull request as ready for review August 11, 2026 15:56

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

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

Nice. Left some questions / comments. Overall looks good

Comment thread docs/assets/contributing/dockerfile-stages-dependency.png
Comment thread docs/features/quantization/online.md Outdated
Comment on lines +103 to +104
checkpoint-specific; today this is wired up for MXFP4 MoE checkpoints
(gpt-oss) where you can opt into FP8 activations:

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.

Probably offtopic, does this only support gpt-oss for now? if not let's extend the description here with other mxfp4 models (kimi k3, deepseek v4) and other quantizers (quark, etc)

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.

Updated, this should read as "weight quantization dtype specific" (weight quant key), this previous doc is outdated.

Note however that some linear/MOE oracles do NOT support activation quant key, see e.g.

def select_mxfp8_moe_backend(

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.

Fixed in 7846c20

) -> Mxfp8LinearKernel:
"""Select and instantiate the best MXFP8 linear kernel for the
current platform."""
if activation_quant_key not in (None, kMxfp8Dynamic):

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.

Is it sound to use None as no change to default? Not sure if other places use None to represent BF16.

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.

also how is the default activation quant key for each quant method documented / decided?

@fxmarty-amd fxmarty-amd Aug 17, 2026

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.

This is a good point. Historically, None represents "no quantization"/BF16, see:

# User-facing names addressable from quantization_config.
QUANT_KEY_NAMES: dict[str, QuantKey] = {
"fp8_per_tensor_static": kFp8StaticTensorSym,
"fp8_per_tensor_dynamic": kFp8DynamicTensorSym,
"fp8_per_token": kFp8DynamicTokenSym,
"fp8_per_channel_static": kFp8StaticChannelSym,
"fp8_per_block_static": kFp8Static128BlockSym,
"fp8_per_block_dynamic": kFp8Dynamic128Sym,
"mxfp8": kMxfp8Dynamic,
"mxfp4": kMxfp4Dynamic,
"int8_per_channel_static": kInt8StaticChannelSym,
}

However we do need to distinguish between a default None (currently on main):

@config
class QuantSpec:
"""Quantization spec for one layer kind (linear or MoE).
`None` on either side means the method class falls back to its own default
(typically inherited from the checkpoint, or unquantized for online).
"""
weight: QuantKeyField = None

and user-set --quantization-config {"linear": {"activation": "null"}}, this is why weight: QuantKeyField = _UNSET, activation: QuantKeyField = _UNSET is introduced in this PR, and used in https://github.com/fxmarty-amd/vllm/blob/7846c2061c808d9757b550d3d2358ff9694fb40e/vllm/config/quantization.py#L97-L110

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.

any reason not to create a dedicated enum for unset?

@mergify

mergify Bot commented Aug 21, 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, @fxmarty-amd.

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 21, 2026
Signed-off-by: Felix Marty <Felix.Marty@amd.com>
@fxmarty-amd
fxmarty-amd force-pushed the relax-online-quant-act-key-override branch from 2531578 to 6e18cbc Compare August 21, 2026 15:35
@mergify mergify Bot added the nvidia label Aug 21, 2026
@AndreasKaratzas AndreasKaratzas added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 21, 2026
@github-actions

Copy link
Copy Markdown

@fxmarty-amd, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@mergify mergify Bot removed the needs-rebase label Aug 21, 2026

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

LG overall. I have some small comments and a question about UNSET and default.

) -> Mxfp8LinearKernel:
"""Select and instantiate the best MXFP8 linear kernel for the
current platform."""
if activation_quant_key not in (None, kMxfp8Dynamic):

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.

any reason not to create a dedicated enum for unset?

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.

merge issue? unrelated changes and many other files with log2_lse_to_ln function

Comment on lines -447 to +497
weight_key: "QuantKey | None" = None,
activation_key: "QuantKey | None" = None,
activation_key: "QuantKey | None",
weight_key: "QuantKey",

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.

why order swap?

@mergify

mergify Bot commented Aug 22, 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, @fxmarty-amd.

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 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation needs-rebase nvidia quantization ready ONLY add when PR is ready to merge/full CI is needed

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants