Repository navigation
Conversation
Load block-32 uint8 E8M0 scales through init_mxfp4_linear_kernel and keep the BF16 rank correction. NVFP4 remains SM103-only; MXFP4 is accepted on SM100/SM103. Signed-off-by: yiliu30 <yi4.liu@intel.com>
Signed-off-by: yiliu30 <yi4.liu@intel.com>
Signed-off-by: yiliu30 <yi4.liu@intel.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to belong to: docs/design/module/quantization.md, docs/design/module/diffusion/index.md, docs/design/module/diffusion/offloader.md. Module owners: @david6666666 @Isotr0py @lishunyang12 Routing: @david6666666 via module of the changed files, module named in the PR description, model owner, CODEOWNERS; @Isotr0py via module of the changed files, module named in the PR description; @lishunyang12 via module of the changed files, module named in the PR description @yiliu30, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
|
Hi @lishunyang12 @david6666666 @xuechendi PTAL, thx! |
david6666666
left a comment
There was a problem hiding this comment.
Two Marlin compatibility issues in the new MXFP4 path.
|
|
||
| if precision == "mxfp4": | ||
| kernel = _mxfp4_kernel() | ||
| kernel.process_weights_after_loading(layer) |
There was a problem hiding this comment.
Marlin permutes and may pad layer.bias here, but apply() passes bias=None to the kernel and adds that packed bias directly to the output afterward. This gives wrong values for biased MXFP4 layers, or a shape error when N needs padding. Could we preserve the original bias for that final add, or let Marlin apply it?
There was a problem hiding this comment.
Good catch. Fixed by preserving the logical bias before MXFP4 post-load processing, then using it for the final add after the rank correction. Added a CPU regression plus a B200 ReplicatedLinear test forced through Marlin with padded N (96 → 128) and a biased forward.
There was a problem hiding this comment.
[P2] Preserve the caller's bias suppression when using the saved Marlin bias
The original packed-bias problem is addressed, but at 573a73c67b0ed04a3fb621bb6221dfb65afe9e9c, svdquant_config.py:445-447 now adds _svdquant_bias even when the caller explicitly passes bias=None. In vLLM 0.32.0, RowParallelLinear.forward() passes None on every TP rank except rank 0 before all-reducing the outputs (linear.py:1775-1781). Marlin replaces the bias on every rank, so this path adds it on every rank and produces sum(partials) + TP * bias instead of sum(partials) + bias. This is reachable for biased quantized row-parallel projections such as FLUX to_out.0 (flux_transformer.py:165-172).
The same ownership issue breaks skip_bias_add=True: the method adds bias despite None, while the linear wrapper returns the still-permuted/padded layer.bias for the caller to add separately. Please preserve both the caller's decision whether to add bias and the logical bias returned by the wrapper. Add coverage for a biased RowParallelLinear at TP=2 and for skip_bias_add=True; the new TP=1 ReplicatedLinear test cannot exercise either case.
This is a source-verified follow-up to this thread; I did not execute PR code or claim a hardware reproduction.
There was a problem hiding this comment.
Good catch. Fixed in 2dc0772: MXFP4 processing now restores the checkpoint bias onto layer.bias after Marlin repacking, and apply() adds bias only when the caller supplies one. This preserves TP>1 row-parallel behavior and skip_bias_add=True while keeping the logical bias shape. Added regressions for a biased TP=2 RowParallelLinear and skip_bias_add=True; 14 focused tests, 36 SVDQuant/H3 tests, and the B200 Marlin replicated-linear test pass. Pre-commit and mypy are clean.
| layer.register_parameter("wtscale", wtscale) | ||
|
|
||
| del input_size, output_size | ||
| layer.output_size_per_partition = output_size_per_partition |
There was a problem hiding this comment.
Please also set layer.input_size_per_partition. ReplicatedLinear doesn't define it, but Marlin's prepare_fp4_layer_for_marlin() reads it, so MXFP4 loading fails for replicated projections such as FLUX's proj_mlp. A loading test with a real ReplicatedLinear would catch this.
There was a problem hiding this comment.
Good catch. create_weights() now sets layer.input_size_per_partition. The new B200 regression constructs a real ReplicatedLinear, forces Marlin, and exercises the loading and forward path that previously failed.
66b43dc to
6df2710
Compare
Signed-off-by: yiliu30 <yi4.liu@intel.com>
|
Re-requesting review on the current head |
|
Thanks @yiliu30. The MXFP4 layout, kernel dispatch, Marlin bias fix and H3 grouped-QKV reorder for the serialized tensors all look correct to me. Comments:
XPU regression check at 6df2710 against merge-base 81d8516, on the same B70 setup.
|
|
Good catches, thanks @xuechendi.
Updated in |
Signed-off-by: yiliu30 <yi4.liu@intel.com>
|
Thanks @yiliu30, 573a73c addresses all four of my points. I re-ran on Intel Arc (B70, torch 2.14.0+xpu, vLLM 0.32) at this head with no gate bypass: the SVDQuant MXFP4 +1 to @Bounty-hunter's point on |
Signed-off-by: yiliu30 <yi4.liu@intel.com>
|
Hi @xuechendi @Bounty-hunter @david6666666 @hsliuustc0106 PTAL, thanks! |
Contributor intent (required human answers)
Public-safe summary based on the contributor's explicit request to upstream the
serialized SVDQuant MXFP4 support.
1. What outcome do you want to achieve?
Enable vLLM-Omni users to load and run serialized SVDQuant MXFP4 diffusion
checkpoints directly, including the public MiniMax-H3 FL2VA checkpoint, without
fork-local runtime changes.
2. What is the highest-leverage variable for that outcome?
Correct end-to-end handling of the serialized MXFP4 checkpoint contract: kernel
selection, block-32 E8M0 scales, tensor-parallel loading, and the H3 grouped-QKV
row order.
3. How will this work move that variable?
This PR adds MXFP4 to the SVDQuant config and linear path and applies the same
grouped-QKV to Q/K/V permutation used by BF16 H3 weights to every serialized
quantized tensor before vLLM's fused QKV loader runs. It then validates both
FlashInfer W4A4 and Marlin W4A16 execution with coherent H3 video.
4. Whose customer or user needs does this address, and what are those needs?
Users with offline MXFP4 SVDQuant diffusion checkpoints on NVIDIA Blackwell need
those checkpoints to load without silent QKV corruption, preserve the low-rank
correction, and run with the best available MXFP4 kernel on one GPU.
Purpose
Before this PR, the serialized SVDQuant path accepted only NVFP4, and
MiniMax-H3 did not reorder serialized QKV tensors. The latter caused the fused
QKV loader to consume grouped rows as Q/K/V, producing noise.
After this PR:
DiffusionSVDQuantConfigacceptsprecision="mxfp4"; NVFP4 behavior isunchanged.
qweightuint8 [N, K/2]and E8M0wscalesuint8 [K/32, N];wtscaleandwcscalesare not registered.process_weights_after_loading()delegates toinit_mxfp4_linear_kernel(activation_quant_key=kMxfp4Dynamic)and retains therank correction in BF16.
qkv_projtensors before weight loading.Affected documentation:
docs/user_guide/quantization/svdquant.md.Test Plan
vLLM Version: 0.32.0
vLLM-Omni Commit:
2dc0772f552f080f84aee5710b007379e5f0dbd8Hardware: NVIDIA B200, SM100, 183 GiB, one GPU
Software: PyTorch 2.13.0, FlashInfer 0.7.0.post1
pytest tests/diffusion/quantization/test_svdquant_config.py \ tests/diffusion/quantization/test_svdquant_linear.py \ tests/diffusion/quantization/test_svdquant_tp_loading.py -q pytest tests/diffusion/models/minimax_h3/test_minimax_h3_quantization.py -q # Bias-ownership follow-up pytest tests/diffusion/quantization/test_svdquant_linear.py -q CUDA_VISIBLE_DEVICES=4 pytest tests/diffusion/quantization/test_svdquant_marlin_gpu.py -q uvx pre-commit run --files \ docs/user_guide/quantization/svdquant.md \ tests/diffusion/models/minimax_h3/test_minimax_h3_quantization.py \ tests/diffusion/quantization/test_svdquant_config.py \ tests/diffusion/quantization/test_svdquant_linear.py \ tests/diffusion/quantization/test_svdquant_tp_loading.py \ vllm_omni/diffusion/models/minimax_h3/minimax_h3_transformer.py \ vllm_omni/quantization/svdquant_config.pyTest Result
reorder regression.
mypy-3.10, ruff, and markdownlint.
git diff --check: clean.2dc0772: SVDQuant linear tests 14 passed,including TP=2
RowParallelLinearbias-once andskip_bias_add=Trueregressions.
tests: 36 passed.
the restored logical
(96,)bias.End-to-end MiniMax-H3 FL2VA validation used the model-card request
(1344x768, 243 frames, 24 fps, 10 seconds, 50 steps, seed 0, Cache-DiT):
Both outputs are coherent and follow the BF16 scene, subject, and motion. The
PSNR/SSIM values are low because the diffusion trajectory diverges after
quantization; they are not reconstruction metrics.
Reproduction commands, inputs, and generated media are documented in the public
model card:
https://huggingface.co/Yi30/MiniMax-H3-FL2VA-SVD-MXFP4
sample_t2va_768p_mxfp4_w4a4
Checks not run:
iters=0RTN residuals.AI assistance
Codex assisted with implementing the runtime changes, tests, documentation, and
drafting this description. All listed commands and results were executed. The
contributor requested this upstream submission and remains responsible for
review and responses to maintainer feedback.
Need help on this PR? Tag
@codesmith-botwith what you need.