Skip to content

fix: surface clear error for missing parameters in LinearBase weight loader - #53118

Open
Tejas-Raj01 wants to merge 1 commit into
vllm-project:mainfrom
Tejas-Raj01:fix-linear-weight-error
Open

fix: surface clear error for missing parameters in LinearBase weight loader#53118
Tejas-Raj01 wants to merge 1 commit into
vllm-project:mainfrom
Tejas-Raj01:fix-linear-weight-error

Conversation

@Tejas-Raj01

@Tejas-Raj01 Tejas-Raj01 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Resolves #53107

Purpose

When LinearBase.load_weights encounters an unexpected tensor in a checkpoint (such as a quantization scale on an unquantized layer), the parameter lookup falls back to self (the module itself). This previously allowed the loading loop to proceed until it crashed several frames later inside the weight loader with a confusing AttributeError: 'MergedColumnParallelLinear' object has no attribute 'data'.

This PR fixes the issue by explicitly verifying that the resolved param is a torch.nn.Parameter. If it is not, the loader now intercepts it and raises a clear, descriptive ValueError indicating exactly which parameter is missing and what type was found instead.

Test Plan

  • Created a new unit test at tests/model_executor/layers/test_linear_missing_param.py that utilizes a dummy layer and binds the vLLM load_weights method to safely test the loading loop without distributed GPU state.
  • Run pytest tests/model_executor/layers/test_linear_missing_param.py -v to verify the new error handling.

Test Result

  • Unit test passes successfully.
  • Attempting to load an undeclared weight now immediately halts with a clean message (e.g., ValueError: dummy_layer: cannot load 'unexpected_scale' — no such parameter, got DummyLayer instead...) rather than a cryptic AttributeError.

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.

…loader

Signed-off-by: Tejas-Raj01 <rajtejas.xyz@gmail.com>

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

@afierka-intel

Copy link
Copy Markdown
Contributor

I had the same fix sitting in my fork (found while debugging GPTQ MoE loading on Intel XPU) and am closing it rather than opening a competing PR — yours resolves #53107 and got here first. Two things from my side that may be useful.

A real checkpoint that reproduces it, if you want something beyond the dummy-layer test: shanjiaz/gemma4-dflash-speculator-fp8-block.

vllm serve google/gemma-4-31B-it \
  --speculative-config '{"model":"shanjiaz/gemma4-dflash-speculator-fp8-block","num_speculative_tokens":7}'

Its quantization_config (compressed-tensors, float-quantized, strategy: block, block_structure: [128, 128]) sits at the top level of config.json, while transformer_layer_config has none. SpeculatorsConfig.extract_transformers_pre_trained_config builds the draft config from transformer_layer_config only (speculators/base.py:61), and no algo_updater in speculators/algos.py copies quantization_config across — grep there returns zero hits. So the draft linears are constructed with quant_config=None while model.safetensors.index.json carries 36 weight_scale tensors (layers.N.mlp.{gate,up,down}_proj.weight_scale, fc.weight_scale), and every one of them lands on the sentinel.

That is arguably a second bug worth its own fix — the draft model silently loses its quantization — but your change is what turns it from an opaque AttributeError into a message naming the tensor, which is the part #53107 asks for.

One difference in approach, take it or leave it. I removed the getattr(..., self) sentinel outright rather than type-checking after it, so the resolution returns None and an explicit allowlist decides what may be skipped:

_OPTIONAL_CHECKPOINT_TENSORS = frozenset(("bias",))

The reason I went that way: isinstance(param, Parameter) also rejects a name that resolves to a registered non-Parameter attribute — a buffer, say — where the current name == "bias" special case would not. I have no checkpoint that hits it, so it is not a defect report, just the case that pushed me to an allowlist instead of a type check.

I also have 129 lines of tests for these two call sites that ran on real hardware (Intel B70 and NVIDIA H200, 3 failed → 5 passed on both, identical split, which is what convinced me linear.py is platform-independent here). Happy to hand them over for this PR if you want them — say the word and I will post the file or open a PR against your branch.

@Tejas-Raj01

Copy link
Copy Markdown
Contributor Author

Hi @afierka-intel,

thanks for stopping by and sharing your findings! That explanation regarding the top-level quantization_config getting dropped in SpeculatorsConfig is super helpful and explains why those scale tensors are left hanging.

Your point about isinstance(param, Parameter) potentially catching registered non-Parameter attributes (like buffers) is also a great catch. Using an explicit allowlist for things like "bias" is much cleaner. If you're willing to share that test file or open a PR/snippet against my branch, I'd gladly integrate it!

Also, regarding the failing pre-commit / pre-run-check, I'm syncing up with the latest main right now to clear that out. Thanks again!
_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: LinearBase.load_weights substitutes the module for a missing parameter, surfacing as a confusing AttributeError

2 participants