Repository navigation
[weight loader] add dtype equality validation between parameter and weight - #51792
Conversation
Signed-off-by: Andy Xie <andy.xning@gmail.com>
7be6321 to
73bfada
Compare
simon-mo
left a comment
There was a problem hiding this comment.
AI-assisted review performed on behalf of Simon Mo. Reviewed commit 4ddf99c90af13954bae4c8fd9523127583f1375c.
No blocking findings. The warning is diagnostic only and preserves the existing weight loader, casts, and yielded loaded names. Focused CPU probes executed the unchanged _load_param method and default_weight_loader with real torch parameters for equal and differing dtypes; values and loader behavior were preserved. Non-blocking: intentional checkpoint-to-runtime casts (for example an explicit --dtype choice) also warn once per weight, so consider aggregation or debug-level diagnostics if this becomes noisy. Full model/GPU execution was not run locally; normal CI remains required.
simon-mo
left a comment
There was a problem hiding this comment.
AI-assisted re-review on behalf of Simon Mo at 7ea7766404a2115d5bdbdf6e8738ebe3d02e0ef3. Compared the current diff with the previously reviewed change: identical. Re-ran the focused CPU timing/weight-loader probes against these current-head files (3 passed). Approval remains valid; required upstream CI must pass before merge.
|
/ci run |
|
❌ This PR is 10 commits behind upstream |
simon-mo
left a comment
There was a problem hiding this comment.
AI-assisted review on behalf of Simon Mo after updating the branch against main. Reviewed 6d130b63997d43499807e823b0ac572e2848bef8; the PR diff is byte-for-byte identical to the change already reviewed and CPU-tested. No new blocking findings. Required CI remains the merge gate.
|
/ci run |
|
✅ Triggered Buildkite CI #92264 for commit |
|
AI-assisted CI follow-up from Simon’s agent: both this PR and #52141 are blocked by the same existing-main test-fixture mismatch, not by the logging change. Buildkite 92264 fails all five MiniMax PP cases in I checked base/main vllm_config=SimpleNamespace(
model_config=SimpleNamespace(hf_text_config=config),
quant_config=None,
use_v2_model_runner=False,
speculative_config=None,
)@andyxning could you add this fixture correction (preferably once upstream, then update both branches), and run the five existing PP tests? I’ll re-review the resulting heads, restart CI, and merge once required checks pass. A blind retry won’t resolve these deterministic failures. |
simon-mo
left a comment
There was a problem hiding this comment.
AI-assisted re-review for Simon: refreshed against current main including the MiniMax PP fixture correction (#59508). The PR diff is byte-identical to the previously reviewed logging change. Focused CPU review probes pass; required hosted CI remains the merge gate.
|
/ci run |
|
@andyxning the shared MiniMax PP fixture mismatch is now fixed upstream by #59508. I updated this branch to main, verified the PR diff is unchanged, reran the three focused logging probes, refreshed approval on the new head, and requested a new CI build. No separate fixture fix is needed from you now. I will handle the merge once required CI passes. (AI-assisted follow-up for Simon.) |
|
✅ Triggered Buildkite CI #92307 for commit |
Purpose
Add weight and model parameter tensor dtype mismatch warning log.
For example, with https://huggingface.co/Qwen/Qwen3.5-27B/tree/main?show_file_info=model.safetensors.index.json,
layers.9.linear_attn.norm.weightis usingF32but the qwen 3.5 model definition used in vllm declare the norm parameter as qwen 3.5 config.json default dtypebfloat16in:vllm/vllm/model_executor/layers/mamba/gdn/qwen_gdn_linear_attn.py
Lines 462 to 469 in 3fb7bb4
This mismatch may cause effect problem.
Us
warninginstead ofwarning_oncebecause in case pipeline parallelism is used, difference layers are placed in difference rank in pp group. We can not gurantee only first rank in pp group has mismatch layers.Test Plan
Tested with Qwen3.5-27B, before the pr the loading log shows nothing about the mismatch.
After the pr the loading log shows that:
This PR share same background with #51836.
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.