Skip to content

[Bugfix] Enhance extra_config handling for layer name suffix matching - #48589

Merged
jikunshang merged 7 commits into
vllm-project:mainfrom
xin3he:fix_inc
Jul 28, 2026
Merged

jikunshang merged 7 commits into
vllm-project:mainfrom
xin3he:fix_inc

Conversation

@xin3he

@xin3he xin3he commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

This pull request improves how the configuration is selected for layers with extra configuration by adding support for suffix matching. This addresses situations where the keys in extra_config are short names (like "lm_head"), but the actual layer_name is fully qualified (like "model.language_model.lm_head"), which can happen due to model nesting.

Configuration handling improvements:

  • Added suffix matching for layer_name when looking up keys in extra_config, so that short names in the config will correctly apply to fully qualified layer names. This ensures that extra configuration is applied even when model nesting changes the layer naming.

Purpose

Support MLLM quantized lm_head in inc/auto-round format, related issue: intel/auto-round#1709
@Yi4Liu @wenhuach21

Test Plan

  • test_inc_config_parser_suffix_match_for_lm_head
  • test_inc_get_quant_method_lm_head_uses_suffix_match

Real model: quantize and load Qwen/Qwen3.5-9B.
Quantization conmmand: auto-round /models/Qwen3.5-9B/ --quant_lm_head --iters 0 --disable_opt_rtn
Serve command: vllm serve tmp_autoround/ --port 8000 --tensor-parallel-size 1 --max-model-len 2048 --reasoning-parser qwen3 --served-model-name qwen

Test Result

UT passed.
Model loaded successfully.


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.

@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 bug Something isn't working label Jul 14, 2026
xin3he added 2 commits July 14, 2026 16:45
…nfig parser

Signed-off-by: Xin He <xin3.he@intel.com>

cloudintheskyfield commented Jul 20, 2026

Copy link
Copy Markdown

This PR remains the minimal fix for #49137: it correctly handles the reported direct lm_head short-key / nested-runtime-name case.

A deeper audit found several adjacent AutoRound name-resolution paths that are outside this PR current code and tests. I prepared and locally validated a stacked follow-up that:

  • centralizes exact, longest component-boundary suffix, bounded literal (for example .experts.), and regex matching;
  • applies the same lookup to packed constituent names such as q/k/v and to RoutedExperts descendant configs;
  • preserves exact-key priority over the model. fallback in get_quant_method;
  • replaces the sentinel lm_head dispatch assertion with an observable ParallelLMHead parameter-registration test (qweight rather than weight).

Against commit 44cf86362, seven added cases fail: longest-suffix selection, exact-vs-model fallback, packed short-name resolution/consistency, FusedMoE short descendants/consistency, and the AutoRound bounded .experts. pattern. On the stacked patch:

  • AutoRound non-model unit tests: 37 passed, 2 deselected
  • pre-commit on the three changed files: all passed
  • a 1,920-key / 10-layer FusedMoE matcher check completes in about 0.10 s after removing an O(N^2) scan found during independent review

@xin3he, would you like this as a stacked PR targeting your fix_inc branch, or would you prefer to keep #48589 minimal and handle the broader resolver separately? I will not publish the patch until the submitting human has completed the repository-required line-by-line review and local test run.

Copy link
Copy Markdown

@xin3he The stacked patch is now published after the repository-required human line-by-line review and personal test run:

The commit is based directly on 44cf8636213adb424eb1e357d298d1545ed5c0ec; no competing upstream PR has been opened. Validation remains:

  • AutoRound non-model tests: 37 passed, 2 deselected
  • all pre-commit hooks for the three touched files passed
  • the seven broader resolver cases fail on 44cf86362 and pass on the stacked commit

Could you either run your Qwen3.5-9B --quant_lm_head load against this commit, or let me know whether you prefer a draft stacked PR targeting xin3he/vllm:fix_inc? I have also asked the #49137 reporter to validate the 27B checkpoint on 2×RTX 3090.

Copy link
Copy Markdown

Update: the stacked follow-up is now available as a draft PR:

It targets xin3he/vllm:fix_inc, is exactly one commit ahead of the current 44cf86362 head, and remains Draft while real-model revalidation is pending. @xin3he, feedback on the resolver scope and the Qwen3.5-9B revalidation can go directly on the stacked PR.

@xin3he

xin3he commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@jikunshang Could you please help review this PR?

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

LGTM, please add model level test, thanks!

@xin3he

xin3he commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Model loaded successfully.

The model level test is executed locally as described above, it's a bug fix so I only updated the UT to cover.

@yiliu30

yiliu30 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Model loaded successfully.

The model level test is executed locally as described above, it's a bug fix so I only updated the UT to cover.

Great, we already have a model test in our unit tests. Please check the test_auto_round_model.
We can extend it to include a tiny model with a quantized LM head.

@jikunshang jikunshang added ready ONLY add when PR is ready to merge/full CI is needed intel-gpu Related to Intel GPU labels Jul 22, 2026
@mergify

mergify Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Hi @xin3he, the pre-commit checks have failed. Please run:

uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

@mergify mergify Bot added the quantization label Jul 23, 2026
@xin3he

xin3he commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Model loaded successfully.

The model level test is executed locally as described above, it's a bug fix so I only updated the UT to cover.

Great, we already have a model test in our unit tests. Please check the test_auto_round_model. We can extend it to include a tiny model with a quantized LM head.

lm_head is usually tied and cannot be quantized for tiny model, so I will skip it.

@xin3he

xin3he commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Hi @xin3he, the pre-commit checks have failed. Please run:

uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

I ran these commands, but nothing changed.

@jikunshang
jikunshang merged commit 948107a into vllm-project:main Jul 28, 2026
106 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working intel-gpu Related to Intel GPU quantization 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.

4 participants