Skip to content

[Bugfix][Model] Fix FP8 PLE loading in mixed ModelOpt checkpoints - #54882

Merged
vadiklyutiy merged 3 commits into
vllm-project:mainfrom
sychen52:release/qwen38next
Sep 3, 2026
Merged

vadiklyutiy merged 3 commits into
vllm-project:mainfrom
sychen52:release/qwen38next

Conversation

@sychen52

@sychen52 sychen52 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Select the FP8 PLE embedding method from per-layer mixed-precision metadata so NVFP4 checkpoints carrying FP8 PLE weights register and load the global weight scale.

Purpose

Enable Qwen3.8-Flash-Next-NVFP4 checkpoint

Test Plan

added unittest
tested by running checkpoint

Test Result

passed

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

Summary by CodeRabbit

  • New Features

    • Added support for FP8 quantization of Qwen4 experimental PLE n-gram embeddings when using mixed-precision quantization configurations.
  • Tests

    • Added coverage confirming FP8 embedding support for mixed-precision configurations and ensuring unrelated NVFP4 layers remain unsupported.

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

@mergify mergify Bot added the qwen Related to Qwen models label Sep 2, 2026
@mergify

mergify Bot commented Sep 2, 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, @sychen52.

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 Sep 2, 2026
@vadiklyutiy vadiklyutiy added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

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

Select the FP8 PLE embedding method from per-layer mixed-precision metadata so NVFP4 checkpoints carrying FP8 PLE weights register and load the global weight scale.

Signed-off-by: Shiyang Chen <shiychen@nvidia.com>
@sychen52

sychen52 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #86902 for commit bac9c8e57daa.

@sychen52 sychen52 changed the title Fix FP8 PLE loading in mixed ModelOpt checkpoints [Bugfix][Model] Fix FP8 PLE loading in mixed ModelOpt checkpoints Sep 2, 2026
@mergify mergify Bot added bug Something isn't working and removed needs-rebase labels Sep 2, 2026
@sychen52

sychen52 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

✅ Queued 2 failed job(s) for retry in Buildkite CI #86902.

@vadiklyutiy

Copy link
Copy Markdown
Member

/ci retry

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #86968 for commit dd08753a142b, running 2 failed step(s) from Buildkite CI #86902.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 2f43a998-2001-43d6-87d3-d08aeeb4915a

📥 Commits

Reviewing files that changed from the base of the PR and between ad127d9 and dd08753.

📒 Files selected for processing (2)
  • tests/models/qwen4_exp/test_ple.py
  • vllm/models/qwen4_exp/nvidia/ple_layer.py

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The PLE embedding quant-method selector now supports ModelOpt mixed-precision configurations. Tests verify FP8 selection for the PLE prefix and no method selection for an NVFP4 prefix.

Changes

Qwen4Exp PLE mixed-precision support

Layer / File(s) Summary
Mixed-precision selection and validation
vllm/models/qwen4_exp/nvidia/ple_layer.py, tests/models/qwen4_exp/test_ple.py
The selector returns Qwen4ExpPLEFp8EmbeddingMethod when a ModelOptMixedPrecisionConfig resolves the prefix to FP8. Tests verify that an NVFP4 prefix returns None.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to dd087

The change enables FP8 PLE weights to load correctly from mixed-precision Qwen checkpoints while preserving existing behavior for other layer assignments. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: gau-nernst, peakcrosser7

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fixing FP8 PLE loading for mixed ModelOpt checkpoints.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sychen52

sychen52 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

✅ CI is already running for this commit: https://buildkite.com/vllm/ci/builds/86968

@sychen52

sychen52 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Queued 3 failed job(s) for retry in Buildkite CI #86968.

@vadiklyutiy

Copy link
Copy Markdown
Member

/ci retry

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87046 for commit 6259b5efb6ef, running 2 failed step(s) from Buildkite CI #86968.

@sychen52

sychen52 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ CI is already running for this commit: https://buildkite.com/vllm/ci/builds/87046

chawasit added a commit to chawasit/vllm that referenced this pull request Sep 3, 2026
…ors target group

Mixed checkpoints (INT4 experts via compressed-tensors) can carry the PLE
n-gram table as FP8-e4m3 with one global scale, the same layout the official
FP8 checkpoint ships and that PLE CPU offload keeps resident at 1 byte/param
(~51 GB instead of ~102 GB host RAM for Qwen3.8-Flash-Next).  Mirrors vllm-project#54882
for ModelOptMixedPrecisionConfig: a config group with an FP8 per-tensor
weight scheme whose target matches the embedding prefix selects
Qwen4ExpPLEFp8EmbeddingMethod; anything else keeps the BF16 embedding.
@vadiklyutiy
vadiklyutiy merged commit d4d703c into vllm-project:main Sep 3, 2026
15 checks passed
@Davan-Etelamaki

Copy link
Copy Markdown

This fixed the ModelOptMixedPrecisionConfig case for checkpoints that enumerate the PLE table in quantized_layers with {"quant_algo": "FP8"} (the convention in the added test, and in nvidia/Qwen3.8-Flash-Next-NVFP4). There's a residual gap for the other valid convention.

After this merge, _get_ple_embedding_quant_method() does:

if isinstance(quant_config, ModelOptMixedPrecisionConfig):
    if quant_config._resolve_quant_algo(prefix) == "FP8":
        return Qwen4ExpPLEFp8EmbeddingMethod()
    return None

If a mixed-precision checkpoint keeps its FP8 PLE table in the exclude / ignore list rather than enumerating it in quantized_layers (the convention RadixArk/Qwen3.8-Flash-Next-NVFP4 uses for the PLE table — *.ple.* in ignore — and which carries over when a checkpoint is extended to MIXED_PRECISION while leaving the PLE handling untouched), _resolve_quant_algo(prefix) does not return "FP8" for it. The function returns None, the default BF16 VocabParallelEmbedding path takes over, and you're back to the two symptoms this PR set out to fix: ValueError: There is no module or parameter named 'ngram_embedding.weight_scale' on the offload worker, or (if that's patched) the ~2× host-RAM PLE table from BF16 allocation of an FP8 table.

The Fp8Config branch right below already handles the excluded case explicitly (is_layer_skipped(...)None is the intended answer there because the whole model is FP8). For MixedPrecisionConfig the excluded-but-still-FP8-on-disk case is real. One way to cover it without reworking checkpoint configs — read the checkpoint's own declared dtype:

if isinstance(quant_config, ModelOptMixedPrecisionConfig):
    if quant_config._resolve_quant_algo(prefix) == "FP8":
        return Qwen4ExpPLEFp8EmbeddingMethod()
    # PLE kept in the exclude list rather than enumerated: trust the
    # checkpoint's declared ple_embedding_dtype.
    if quant_config.is_layer_excluded(prefix) and _ple_dtype_is_fp8(ple_embedding_dtype):
        return Qwen4ExpPLEFp8EmbeddingMethod()
    return None

Happy to send this as a small follow-up PR with a test for the exclude-list convention if it's wanted.


Found with AI assistance (Claude Code) while extending a Flash-Next checkpoint to mixed-precision; the fall-through was verified by reading the merged _get_ple_embedding_quant_method against the checkpoint's actual config.

zhendonghua added a commit to zhendonghua/sglang that referenced this pull request Sep 9, 2026
… experts in ModelOpt mixed checkpoints

`nvidia/Qwen3.8-Flash-Next-NVFP4` is a three-precision MIXED_PRECISION
checkpoint: NVFP4 routed experts, an FP8 PLE n-gram table (F8_E4M3
shards + a scalar weight_scale) and block-FP8 MTP experts. Two decisions
were made before consulting the checkpoint's per-layer `quantized_layers`
map and broke on it:

1. PLE table dtype. The table's storage dtype is fixed at construction
   and only switched to fp8 for a whole-checkpoint `fp8` quant name or an
   explicit `text_config.ple_embedding_dtype`. This checkpoint sets
   neither (RadixArk's NVFP4 repo sets the config key; nvidia's does not),
   so the table was built bf16, moved into pinned host memory by the
   default `--ple-offload-embedding`, and load_weights then raised
   "fp8 PLE auto-switch is unsupported with ple_offload_embedding".
   `_ple_table_is_fp8()` now also asks the mixed config's
   `_resolve_quant_algo()` for the embedding's own prefix, so the table is
   fp8 from the start (mirrors vllm-project/vllm#54882).

2. Draft quantization. `_mtp_quant_config` dropped the quant config for
   every modelopt_mixed checkpoint on the assumption that embedded MTP
   weights are bf16. Here `mtp.layers.0.mlp.experts` is block-FP8, so the
   draft's FusedMoE was built unquantized, the fp8 expert values were cast
   to bf16 without their scales and `weight_scale_inv` was silently
   skipped: the server ran and target accuracy was fine, but MTP accept
   length fell to ~1.6. The draft now keeps the quant config when the
   map lists any `mtp.` layer; every other draft module still resolves to
   bf16 through the same per-layer lookup.

Verified on 4x B300 with the user launch command (TP4, NEXTN 3/1/4, no
--quantization flag): loads, sgl-eval GSM8K (200, thinking) 0.97,
accept length ~3.2-3.6, draft weights 1.53 GB (was 2.47 GB bf16).

Depends on the block-FP8 FusedMoE dispatch in ModelOptMixedPrecisionConfig.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working qwen Related to Qwen models ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants