fix(export): honor sub-model scope_prefix in quant-aware reverse rename (NVBug 6525511) - #2076
Conversation
…25511)
LLaVA PTQ on transformers>=5.12 exported every tensor under a bogus
top-level 'vision_model.' namespace, so vLLM refused the checkpoint with:
ValueError: There is no module or parameter named 'vision_model'
in LlavaForConditionalGeneration
Root cause: transformers collects conversion mappings recursively and tags
each sub-model's transforms with 'scope_prefix' (the sub-module path), then
matches only keys under that prefix -- see WeightTransform._scoped_match.
LlavaForConditionalGeneration therefore carries the vision tower's own
PrefixChange ('add a vision_model. prefix'), scoped to 'model.vision_tower'.
_build_reverse_rules read rev.source_patterns/target_patterns raw and dropped
scope_prefix, so that unanchored rule matched every key in the state dict:
all 758 llava-1.5-13b tensors were moved under 'vision_model.', including
'vision_model.language_model.*' and 'vision_model.lm_head.*'.
RenameRule now carries scope_prefixes and _sub_scoped applies a scoped rule
only to keys under one of its prefixes -- stripping the prefix before the
match and re-attaching it after, mirroring transformers' own semantics. The
same scoping is applied to quant-config module names via
build_reverse_name_mapper so exclude_modules stays aligned with the weights.
_drop_shadowed_prefix_renames now skips scoped rules, which are already
confined to their subtree and cannot reach a sibling namespace.
Verified against the real llava-hf/llava-1.5-13b-hf checkpoint: all 758
in-memory keys now map exactly onto the 758 hub keys (0 missing, 0 spurious)
across all three namespaces (language_model 363, vision_tower 391,
multi_modal_projector 4). Pre-fix, 758/758 were mangled.
Signed-off-by: James Shen <yueshen@nvidia.com>
A transform with scope_prefix == "" must still match every key (transformers treats the empty prefix as an always-matching fallback in _scoped_match). Dropping it from the candidate list would have confined such a rule to base_model_prefix. Does not arise from the current transformers collection path (root models leave scope_prefix None), but keeps the translation faithful. Signed-off-by: James Shen <yueshen@nvidia.com>
|
Follow-up evidence: the exported checkpoint is fully functional, and the residual Same PTQ output loaded in vLLM 0.26 under three configurations:
The decisive data point is the third row: Worth noting for whoever picks up the QA ticket: llava-1.5-13b ships |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe export conversion pipeline applies scoped ChangesScoped reverse conversion
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modelopt/torch/export/quant_aware_conversion.py`:
- Around line 444-448: Update converter-derived rule generation in
_expert_leaf_renames() and _dense_split_rule() to retain and apply the
converter’s non-empty scope_prefix and base_model_prefix values when matching
tensors. Ensure the generated RenameRule and SplitRule use scope-aware matching
so scoped SplitModulelist and Chunk conversions do not affect sibling tensors,
and add regression coverage for sibling tensors.
In `@tests/unit/torch/export/test_quant_aware_conversion.py`:
- Around line 329-330: The local imports of PrefixChange in
tests/unit/torch/export/test_quant_aware_conversion.py at lines 329-330 and
369-370 need brief comments explaining that transformers.core_model_loading is
an optional dependency and pytest.importorskip() guards the tests before
importing it. Add the same rationale at both import sites without changing their
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 20ac793b-448c-4349-a90a-6d5c74cdc9df
📒 Files selected for processing (3)
CHANGELOG.rstmodelopt/torch/export/quant_aware_conversion.pytests/unit/torch/export/test_quant_aware_conversion.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2076 +/- ##
===========================================
+ Coverage 67.17% 78.06% +10.89%
===========================================
Files 521 521
Lines 59857 59885 +28
===========================================
+ Hits 40206 46749 +6543
+ Misses 19651 13136 -6515
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/claude review |
…ports Addresses CodeRabbit review on #2076. 1. Converter-derived rules (_expert_leaf_renames, _dense_split_rule) match by module suffix rather than an anchored pattern, so unlike WeightRenaming they cannot be confined to a sub-model subtree. Surveying 9 architectures (LLaVA, LLaVA-Next, Gemma-3, Gemma-4, Qwen2-VL, Qwen3-VL-MoE, Llama-4-Scout, Mixtral, DeepSeek-V2-Lite) every WeightConverter has scope_prefix=None -- transformers only scopes WeightRenaming/PrefixChange -- so the case is not reachable today. Rather than emit rules that silently reach a sibling namespace if that ever changes, raise QuantConversionUnsupportedError so the caller falls back to in-memory names with a warning. 2. Annotate the two local PrefixChange imports in the tests, naming the reason (optional dependency guarded by pytest.importorskip) per the repo guideline. pytest tests/unit/torch/export: 116 passed, 1 skipped. Signed-off-by: James Shen <yueshen@nvidia.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| scope = getattr(rev, "scope_prefix", None) | ||
| if scope is None: | ||
| return () | ||
| scope_dot = f"{scope}." if scope != "" else "" | ||
| base = getattr(rev, "base_model_prefix", None) or "" | ||
| base_dot = f"{base}." if base != "" else "" | ||
| # Deduplicate while preserving order. An empty candidate is kept: it only arises for | ||
| # ``scope_prefix == ""`` and, matching transformers, acts as the always-matching | ||
| # fallback that applies the pattern to the whole key. | ||
| seen: list[str] = [] | ||
| for c in (base_dot + scope_dot, scope_dot): | ||
| if c not in seen: | ||
| seen.append(c) | ||
| return tuple(seen) |
There was a problem hiding this comment.
[IMPORTANT Compatibility] base_model_prefix + scope_prefix builds a combined prefix, but transformers' _scoped_match uses base_model_prefix as an alternative root, not an additional segment — so a scoped rule can silently stop applying.
What's happening. For the llava case in this PR, scope_prefix = "model.vision_tower" and base_model_prefix = "model", so _scope_prefixes returns:
("model.model.vision_tower.", "model.vision_tower.")
The first candidate model.model.vision_tower. is not a real key prefix for this checkpoint — the model. base is already the leading segment of scope_prefix. It never matches, and the second candidate saves the fix. That is why the tests pass. But the two candidates are tried in order and only the first match wins (_sub_scoped returns on the first startswith), so the ordering only happens to be harmless here.
Why it matters. The reason transformers tries base_model_prefix at all is that a state dict may be keyed with or without the base prefix (the classic model. prepend/strip on PreTrainedModel vs PreTrainedModel.model). Those are two alternative spellings of the same subtree, e.g. vision_tower. vs model.vision_tower. — not model. + model.vision_tower.. Concatenating produces a prefix that matches nothing whenever scope_prefix already begins with the base, and misses the case the fallback exists to cover: a scope path relative to the base model (scope_prefix = "vision_tower", keys model.vision_tower.*). In that configuration this code yields ("model.vision_tower.", "vision_tower.") — correct by luck — but for scope_prefix = "model.vision_tower" with keys spelled vision_tower.*, neither candidate matches and the rule is dropped entirely: the vision tower ships unrenamed, missing its vision_model. segment, and the loader then fails on missing keys rather than bogus ones.
Suggested fix. Emit the base-relative and base-absolute spellings as genuine alternatives rather than a concatenation, e.g.:
scope = getattr(rev, "scope_prefix", None)
if scope is None:
return ()
base = getattr(rev, "base_model_prefix", None) or ""
candidates = [scope]
if base:
# The state dict may be keyed with or without the base-model prefix; these are two
# spellings of the same subtree, not nested scopes.
candidates.append(f"{base}.{scope}" if scope else base)
candidates.append(scope.removeprefix(f"{base}."))
seen: list[str] = []
for c in candidates:
c_dot = f"{c}." if c else ""
if c_dot not in seen:
seen.append(c_dot)
return tuple(seen)Please also confirm the intended precedence against the installed WeightTransform._scoped_match — if transformers tries the base-prefixed form first, keep that order, but the candidates should be alternatives either way.
There was a problem hiding this comment.
Thanks -- I checked this carefully because adopting it would change how we agree with the loader, but I have to decline: the premise does not hold for the installed transformers.
You asked me to confirm the intended precedence against WeightTransform._scoped_match, so here it is verbatim from transformers 5.14.1 (the version in the bug's environment, inside vllm/vllm-openai:v0.26.0):
if self.scope_prefix is not None:
scope_prefix = f"{self.scope_prefix}." if self.scope_prefix != "" else ""
base_model_prefix = f"{self.base_model_prefix}." if self.base_model_prefix != "" else ""
# First, try to match the longest sequence, i.e. base_model_prefix + scope_prefix
if source_key.startswith(base_model_prefix + scope_prefix):
prefix = base_model_prefix + scope_prefix
# Then, try to strip the base_model_prefix, in case we load a ForXXX model from BaseModel weights
elif source_key.startswith(scope_prefix):
prefix = scope_prefix
else:
return None
key_to_match = source_key.removeprefix(prefix)transformers does concatenate -- base_model_prefix + scope_prefix literally, with its own comment calling it "the longest sequence" -- and tries that first, bare scope_prefix second. _scope_prefixes reproduces exactly that, same two candidates in the same order. So ("model.model.vision_tower.", "model.vision_tower.") for the llava case is not accidental; it is what the loader computes, and the first candidate failing to match is the intended path, not a near miss the tests happen to survive.
On the scope_prefix = "model.vision_tower" with keys spelled vision_tower.* scenario: you are right that neither candidate matches and the rule is dropped. But transformers takes return None on the very same input, so the loader would not apply the rename either. Emitting a third scope.removeprefix(f"{base}.") candidate would make ModelOpt rename tensors that transformers would not, which is the one thing this code must never do -- exported names have to match what the loader expects. If that two-branch behavior is wrong it is wrong upstream, and diverging unilaterally would turn a shared limitation into a silent mismatch.
One genuine imprecision you surfaced: the upstream docstring says the prefix may be scope_prefix "with one base_model_prefix level stripped or prepended", while the code only implements prepended. The code is authoritative here, and it is what I mirrored.
Leaving this as-is. Your second comment was a real bug and is fixed in 3ad180d.
| for rule in rules: | ||
| if rule.scope_prefixes: | ||
| kept.append(rule) | ||
| continue |
There was a problem hiding this comment.
[IMPORTANT Compatibility] The scope_prefixes-bypass of _drop_shadowed_prefix_renames regresses the #2032 fix for any rule whose scope resolves to an empty/root prefix.
The interaction. _scope_prefixes deliberately keeps an empty candidate for scope_prefix == "" (per the b3c3dbf commit message, "a transform with scope_prefix == "" must still match every key"). So such a rule has scope_prefixes truthy — e.g. ("model.", "") when base_model_prefix = "model", or ("",) when the base is empty — while _sub_scoped's empty candidate matches every key and applies the pattern to the whole key space, exactly like an unscoped rule.
But this early-continue keys off if rule.scope_prefixes: (non-empty tuple), not off whether the scope actually confines the rule. A root-scoped rule therefore takes the bypass and skips the shadowing guard entirely, even though its effective reach is the full state dict. That is precisely the situation #2032 (NVBug 6525534) added the guard for: test_nested_text_prefix_reverse_does_not_capture_vlm_siblings asserts that a ^model.language_model. reverse is dropped on a parent VLM, and build_reverse_name_mapper(model) returns None. If a transformers version ever tags that nested text conversion with scope_prefix = "" instead of leaving it None, the rule is kept, applied globally, and model.visual.* is rewritten to model.language_model.visual.* — reintroducing the earlier bug with no test covering it.
The docstring justifies the bypass as "a rule carrying scope_prefixes is already confined to its sub-model's subtree by _sub_scoped" — true for a non-empty prefix, false for the empty fallback the previous commit intentionally preserved. The two changes are individually reasonable and mutually inconsistent.
Suggested fix. Gate on effective confinement rather than tuple truthiness — a rule is confined only if every candidate prefix is non-empty:
for rule in rules:
# An empty scope candidate matches every key, so such a rule is NOT confined to a
# subtree and still needs the shadowing heuristic.
if rule.scope_prefixes and all(rule.scope_prefixes):
kept.append(rule)
continueWorth a regression test with scope_prefix = "" on the existing model.visual / model.language_model VLM fixture, asserting siblings are untouched.
There was a problem hiding this comment.
Good catch -- accepted and fixed in 3ad180d.
You are right that the two commits were mutually inconsistent. _scope_prefixes deliberately keeps an empty candidate for scope_prefix == "", _sub_scoped matches that against every key, so the rule has whole-key-space reach -- yet if rule.scope_prefixes: is truthy and it took the bypass. Gating on tuple truthiness rather than on actual confinement was the error.
Applied your suggested gate:
for rule in rules:
# `all(...)` matters: an empty candidate matches every key, so a rule carrying one
# is not actually confined and still needs the check below.
if rule.scope_prefixes and all(rule.scope_prefixes):
kept.append(rule)
continueand corrected the docstring, which claimed confinement for all scoped rules when that only holds for non-empty prefixes.
Also added the regression test you suggested, on the existing model.visual / model.language_model VLM fixture with scope_prefix = "", asserting the sibling vision namespace is untouched -- i.e. that the NVBug 6525534 guarantee still holds by the route you identified. It fails without the all(...) gate.
For the record on reachability: this is not triggerable from the current transformers collection path, which leaves scope_prefix as None for root models (is_root_model = module_name == "" only assigns a scope for non-root sub-models), so no shipping architecture produces scope_prefix == "". But the inconsistency was real and the fix is one line, so it is worth closing rather than arguing about reachability.
pytest tests/unit/torch/export: 117 passed, 1 skipped.
There was a problem hiding this comment.
Claude review — fix(export): honor sub-model scope_prefix in quant-aware reverse rename
Scope reviewed: full PR (3 files: modelopt/torch/export/quant_aware_conversion.py, tests/unit/torch/export/test_quant_aware_conversion.py, CHANGELOG.rst). All three opened, including the pre-existing _drop_shadowed_prefix_renames / _build_reverse_rules context the change interacts with. Note: transformers is not installed in this review environment, so the findings below are traced from the code and the PR body description of WeightTransform._scoped_match rather than executed against the real library — the precedence question in finding 1 is worth confirming against your installed version.
Findings — CRITICAL: 0 · IMPORTANT: 2 · SUGGESTION: 0
The diagnosis and the overall shape of the fix are right. Threading scope through RenameRule and applying it in _sub_scoped is the correct level to fix this at, the PR body is unusually well-evidenced (758/758 keys round-tripping is a strong oracle), and routing build_reverse_name_mapper through the same _apply_rename_rules path so exclude_modules stays aligned with the weights is the right call — that alignment bug is subtle and silent when it goes wrong.
Both findings concern the base_model_prefix / empty-scope handling in _scope_prefixes, not the core scoping mechanism. They do not affect the llava case this PR fixes (traced: for scope_prefix="model.vision_tower", base_model_prefix="model", the bogus first candidate model.model.vision_tower. simply never matches and the second candidate carries the fix), which is why the tests and the end-to-end run pass. They are latent-correctness issues in adjacent configurations.
-
[IMPORTANT Compatibility]
_scope_prefixes(L333–346) —base_model_prefix + scope_prefixbuilds a combined prefix, but transformers usesbase_model_prefixas an alternative root spelling (state dict keyed with vs. without the base prefix), not an extra nested segment. Whenscope_prefixalready starts with the base — the actual llava case — the concatenation yields a prefix matching nothing. In the mirror configuration (scope_prefix="model.vision_tower", keys spelledvision_tower.*) neither candidate matches and the rule is dropped, so the vision tower ships without itsvision_model.segment and the loader fails on missing keys instead of bogus ones. -
[IMPORTANT Compatibility]
_drop_shadowed_prefix_renamesbypass (L368–371) — theif rule.scope_prefixes: continueearly-out keys off tuple truthiness, butb3c3dbfdeliberately keeps an empty candidate forscope_prefix == "", and an empty prefix matches every key. Such a rule is therefore not confined to a subtree yet still skips the #2032 shadowing guard. Gate onrule.scope_prefixes and all(rule.scope_prefixes)instead.
One elaboration shared by both
Both have a common root: whether a conversion regex anchors are written relative to the scope-stripped suffix or to the full key. The fix assumes scope-stripped, correct for llava vision-tower PrefixChange (^(?:(?!vision_model\.))(.+)$ is relative to the tower own root). But combine that assumption with the empty-scope fallback and it turns self-defeating: for a root-owned rule with scope_prefix="", base_model_prefix="model", candidate model. strips the leading model. off model.layers.0.weight, leaving layers.0.weight — against which the ^model\.(?!language_model\.) pattern from test_nested_text_prefix_reverse_still_applies_to_text_model no longer matches, and the rename is silently lost. That test passes today only because the current collection path leaves scope_prefix as None for root models (as the b3c3dbf message notes). The empty-scope fallback added "to keep the translation faithful" is, as written, the one case where the translation is not faithful — either drop it (unreachable via the current path) or make it bypass prefix-stripping entirely rather than routing through base_model_prefix.
Not duplicated here
CodeRabbit already flagged that converter-derived rules (_expert_leaf_renames, _dense_split_rule) discard the converter scope_prefix — I agree it is a real gap and have not re-raised it. It is lower-risk than the WeightRenaming path, since those rules anchor on .experts.<i>.<leaf> / leaf suffixes rather than ^, so they cannot capture the whole key space the way the llava PrefixChange did.
Risk assessment
Low-to-moderate. The change is well-contained, gated behind the existing QuantConversionUnsupportedError fallback to legacy naming, and fixes a total-corruption bug (758/758 tensors mis-namespaced) with genuine end-to-end validation. Both findings are latent rather than active regressions against the current transformers collection path — but finding 2 failure mode is a silent reintroduction of the #2032 bug, and both are small fixes, so they are worth resolving before merge rather than after.
Addresses Claude review on #2076. _scope_prefixes keeps an empty candidate for scope_prefix == "", and _sub_scoped matches that against every key -- so such a rule has the reach of an unscoped one. _drop_shadowed_prefix_renames gated on the tuple being non-empty, which let a root-scoped rule skip the heuristic entirely and could reintroduce NVBug 6525534 (a nested text model's ^model.language_model. reverse rewriting model.visual.*). Gate on every candidate being non-empty instead, i.e. on the rule actually being confined. Not reachable from the current transformers collection path, which leaves scope_prefix None for root models, but the two guards were mutually inconsistent. Covered by a new regression test. pytest tests/unit/torch/export: 117 passed, 1 skipped. Signed-off-by: James Shen <yueshen@nvidia.com>
The bug is filed against 0.46.0 (Committed_ModelOpt_0.46.0) and the fix targets that release, so the entry belongs in the 0.46 section -- next to the related #2032 nested-VLM-namespace fix -- not 0.47. Also notes that Gemma3ForConditionalGeneration is affected identically and fixed by the same change, as found by the cross-architecture regression sweep. Signed-off-by: James Shen <yueshen@nvidia.com>
cjluo-nv
left a comment
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (gpt-5.6-sol) — DM the bot to share feedback.
The fix correctly carries transformers sub-model scope into reverse rename rules, applies the same scoped mapping to tensor keys and quantization-config names, and safely rejects scoped converter operations that cannot yet be confined. The regression tests cover sibling isolation, wildcard/config mapping, the existing shadowing guard, and unsupported scoped converters. The change is focused, documented in the changelog, and includes meaningful unit and end-to-end validation.
kevalmorabia97
added
the
cherry-pick-done
#2159 #2112 (#2179) ## Cherry-picked PRs - #1975 - #2076 - #2071 - #2093 - #2084 - #2115 - #2133 - #2146 - #2064 - #2159 - #2112 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added PTQ recipes for Nemotron model families and NVFP4 mixer-MLP quantization. * Added Qwen3-VL multimodal speculative-decoding support, including video inputs. * Added compatibility with multiple vLLM KV-cache layouts and newer Transformers versions. * Added a Nemotron 3.5 Lightning quantization-aware distillation workflow. * **Bug Fixes** * Improved Hugging Face, QLoRA, and PEFT checkpoint exports. * Improved distributed job shutdown when a process encounters an error. * Improved pruning validation and candidate selection for MoE models. * **Documentation** * Updated supported-model lists and recipe paths. * Removed Phi-3 Vision and Phi-4 Multimodal quantization support. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Slawomir Kierat <skierat@nvidia.com> Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com> Signed-off-by: James Shen <yueshen@nvidia.com> Signed-off-by: Zhiyu Cheng <zhiyuc@nvidia.com> Signed-off-by: Kinjal Patel <kinjalpravin@nvidia.com> Signed-off-by: Shiyang Chen <shiychen@nvidia.com> Signed-off-by: Chenjie Luo <chenjiel@nvidia.com> Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com> Signed-off-by: Jennifer Chen <jennifchen@nvidia.com> Signed-off-by: Suguna Velury <178320438+sugunav14@users.noreply.github.com> Co-authored-by: skierat <skierat@nvidia.com> Co-authored-by: h-guo18 <67671475+h-guo18@users.noreply.github.com> Co-authored-by: yueshen2016 <39203804+yueshen2016@users.noreply.github.com> Co-authored-by: Zhiyu <zhiyuc@nvidia.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: kinjalpatel27 <31936134+kinjalpatel27@users.noreply.github.com> Co-authored-by: sychen52 <41452870+sychen52@users.noreply.github.com> Co-authored-by: Chenjie Luo <108829653+cjluo-nv@users.noreply.github.com> Co-authored-by: Wei-Ming Chen <17592131+meenchen@users.noreply.github.com> Co-authored-by: Jenny Chen <jennifchen@nvidia.com> Co-authored-by: sugunav14 <178320438+sugunav14@users.noreply.github.com>
3 participants
What does this PR do?
Type of change: Bug fix
Fixes NVBug 6525511 / OMNIML-5599: FP8 PTQ of
llava-1.5-13bontransformers>=5.12produces a checkpoint vLLM refuses to load:A cross-architecture sweep run for this PR shows
google/gemma-3-4b-itis broken the same way (884/884 keys mangled) and is fixed by the same change, though no bug was filed for it.Root cause
transformers collects conversion mappings recursively and tags each sub-model's transforms with the sub-module path they belong to, then matches only keys under that prefix (
WeightTransform._scoped_match: stripscope_prefix→ match → re-attach):LlavaForConditionalGenerationtherefore carries the vision tower's ownPrefixChange— "add avision_model.prefix" — scoped tomodel.vision_tower:_build_reverse_rulesreadrev.source_patterns/rev.target_patternsraw and discardedscope_prefix. Read as an unscoped regex, that pattern means "any key not already starting withvision_model." — i.e. everything. All 758 llava-1.5-13b tensors were moved under a bogus top-levelvision_model.:This is the same class of defect as #2032 (NVBug 6525534), but that fix's shadowing heuristic cannot catch it: it drops a rule whose target is an existing namespace, whereas this rule invents one (
vision_modelexists nowhere in the module tree).The fix
RenameRulecarriesscope_prefixes;_sub_scopedapplies a scoped rule only to keys under one of them — stripping the prefix before the match and re-attaching after, mirroring transformers' own semantics (tryingbase_model_prefix + scope_prefixbeforescope_prefix). The same scoping flows throughbuild_reverse_name_mapper, soexclude_modules(which lists the BF16 vision tower) stays aligned with the weights._drop_shadowed_prefix_renamesskips scoped rules, which are already confined to their subtree.Converter-derived rules (
_expert_leaf_renames,_dense_split_rule) match by module suffix rather than an anchored pattern, so they cannot be confined to a subtree the same way. A survey of 9 architectures (LLaVA, LLaVA-Next, Gemma-3, Gemma-4, Qwen2-VL, Qwen3-VL-MoE, Llama-4-Scout, Mixtral, DeepSeek-V2-Lite) found everyWeightConverterhasscope_prefix=None— transformers only scopesWeightRenaming/PrefixChange— so the case is unreachable today. Rather than emit rules that could silently reach a sibling namespace if that ever changes, a scoped converter now raisesQuantConversionUnsupportedErrorand the caller falls back to in-memory names with a warning.Testing
Name-level oracle against the real hub checkpoint. Exported names must equal the original checkpoint's keys. All 758
llava-hf/llava-1.5-13b-hfstate-dict keys round-trip exactly:Pre-fix, 758/758 keys were mangled. This check involves no vLLM.
Cross-architecture regression sweep — old (unscoped) vs new (scoped) mapping over each model's real state dict:
Note Qwen2-VL carries a scoped rule yet is unchanged — the fix only bites where a scoped rule would have wrongly matched.
End-to-end, on the exact image from the bug report (
vllm/vllm-openai:v0.26.0, verifiedvllm.__version__ == 0.26.0, transformers 5.14.1): real FP8 PTQ (general/ptq/fp8_default-kv_fp8_cast,--calib_size 512) on llava-1.5-13b, then served with the bug's exactapi_servercommand.The reported failure is gone and weight loading completes.
pytest tests/unit/torch/export→ 116 passed, 1 skipped (3 new regression tests)pre-commit run --files ...→ all hooks passAdditional Information
Out of scope, for whoever picks up the QA ticket. After this fix the bug's exact repro hits a different error: llava-1.5-13b ships
"dtype": "float16"and vLLM's FP8 kernel requires BF16 output (RuntimeError: For FP8 input, output must have dtype BF16). This is not an export defect — it also occurs withkv_cache_dtype=auto, i.e. with the FP8 KV cache entirely out of the picture, and the same checkpoint loads and generates correctly under--dtype bfloat16(" Paris. with a population of about 2,249,03"). QA will need--dtype bfloat16.NVBug 6525597 (
gemma-4-31B-it,assert layer.k_scale > 0.0) is unrelated to this PR: Gemma4 exposes zero transformers conversions, so this change provably does not touch it (confirmed identical in the sweep above). Re-tested separately on vLLM 0.26.0 with that bug's environment (transformers 5.5.0, TP=1, batch_size 8) it did not reproduce — the server reachedApplication startup completewith zero asserts — but that is tracked outside this PR.Before your PR is "Ready for review"
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests