Repository navigation
[Bugfix][RL] Load attention sinks through weight_loader for layerwise reload - #57797
Conversation
|
Documentation preview: https://vllm--57797.org.readthedocs.build/en/57797/ |
e79ba24 to
345eb50
Compare
Checkpoint-backed attention sink parameters are loaded with a bare copy_ in
several model load_weights implementations:
param.data.copy_(narrow_weight)
params_dict[name][:n].copy_(narrow_weight)
That write bypasses param.weight_loader, which is the only entry point the
online weight reload path can observe. During RL / online weight update /
layerwise reload the parameter is on meta, so the write lands on the parameter
instance the reload materialized and the framework has no record that the sink
was loaded. Measured through a real layerwise reload cycle, the restored kernel
tensor then holds unwritten storage rather than the new value, and the -inf
padding tail is not re-established either.
Add a design doc specifying the contract (every checkpoint-backed,
online-updatable parameter loads through the parameter's current
weight_loader), the measured failure mode, the 20 confirmed sites across 19
files, the shared padded-sink helper for DeepSeek, and the staged
implementation plan.
Implementation PRs: vllm-project#57797, vllm-project#57798, vllm-project#57799, vllm-project#57800, vllm-project#57801, vllm-project#57803, vllm-project#57804,
vllm-project#57805, vllm-project#57806.
Co-authored-by: AI Assistant <noreply@example.com>
a8d2888 to
fc3085c
Compare
14dfbc2 to
64c77ee
Compare
Verification on H20 (attention-sink series)Verification of this PR used an integrated tree containing all six fix PRs, so the shared helper and every call site were exercised together. Verified commit: Environment
Results
GPU end-to-end output (real layerwise reload on device storage): Command: Pre-commit
Not verified
What changed in this revision
|
64c77ee to
c7b6346
Compare
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #94644. |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #94644. |
|
/ci run |
|
✅ Triggered Buildkite CI #94710 for commit |
Signed-off-by: Ronald1995 <ronaldautomobile@163.com>
|
/ci run |
|
✅ Triggered Buildkite CI #94732 for commit |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #94732. |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #94732. |
… reload (vllm-project#57797) Signed-off-by: Ronald1995 <ronaldautomobile@163.com> Co-authored-by: aoshen02 <aoshen@inferact.ai> Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
…ct#57797 (vllm-project#61678) Signed-off-by: Stefano Castagnetta <scastagnetta@nvidia.com> Co-authored-by: Misha Goin <mgoin64@gmail.com> Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
… reload (vllm-project#57797) Signed-off-by: Ronald1995 <ronaldautomobile@163.com> Co-authored-by: aoshen02 <aoshen@inferact.ai>
…ct#57797 (vllm-project#61678) Signed-off-by: Stefano Castagnetta <scastagnetta@nvidia.com> Co-authored-by: Misha Goin <mgoin64@gmail.com>
Overview
Attention sink writes bypassed parameter
weight_loadercallbacks, leaving stale sink values after layerwise reload during RL weight updates. Register checkpoint-backed sinks with a common TP loader and factory, and route model loading through the registered callbacks.This consolidates and supersedes #57798, #57801, #57803, #57804, #57805, and #57806.
Claims
-inf. The factory initializes sinks to-infand registers the loader.Validation
Current review revision:
7964a96f87Merged upstream
mainat61d9c777bfbefore committing the review fixes. Local Windows / Python 3.12 / PyTorch 2.13.0+cpu:git diff --check, SPDX, forbidden-import and test-tethering checks passed.pytest tests/model_executor/test_weight_utils.py tests/model_executor/model_loader/test_reload.py -k sink -qremains blocked during startup by missinguvloopon Windows. Full pre-commit could not initialize its environments because the configured local proxy is unreachable; commit hooks were bypassed for this revision after the checks above.Historical H20 validation before this review revision
Previous revision
Environment: H20 Slurm cluster, node
vllm-h20-02, 2 x NVIDIA H20-3e, driver 580.105.08; Python 3.12.14 / torch 2.13.0+cu130. Compiled extensions come from the per-commit precompiled cu130 wheel built for this PR's base commit (vllm-0.31.1rc1.dev295+gd4e92ccc3) with the branch's Python files overlaid, so Python and extensions are from the same commit without a rebuild.pytest tests/model_executor/test_weight_utils.py tests/model_executor/model_loader/test_reload.py -k sink -qtest_weight_utils.py+test_reload.py, this branchmain(control)run-precommit.sh(every hook the diff touches)DCO passThe failures are model-download / multi-GPU cases that fail identically before and after the change, so they carry no signal about this PR.
Additional out-of-tree verification (harness deliberately kept outside the PR, results in the review comments): each sink model's real attention module was built from tiny dimensions with dummy weights on H20, for TP=1 and TP=2, and driven through the real layerwise reload lifecycle, asserting the per-rank slice, the
-infpadding and the preserved kernel storage; the pre-PR loading shape was reproduced as a control and never received the reloaded sinks.These results were recorded before the current review changes and are not a rerun of the current head.
Details
A dedicated sink loader derives shard length from the checkpoint rather than the runtime parameter: DeepSeek pads its runtime sink, so
sharded_weight_loader(0)would use the wrong shard length. Unpadded sinks use the same loader without reconstructing storage. Checkpoint heads must divide evenly across TP ranks.The loader keeps the divisibility check and relies on tensor slicing and the default loader for incompatible shapes. Empty checkpoints are no longer explicitly rejected. Lifecycle rationale stays here rather than in long helper docstrings.
Tests in the existing CI-selected
test_reload.pyandtest_weight_utils.pysuites cover actual model dispatch for MiMo MTP/Omni MTP, Laguna, DFlash, GPT-OSS, DeepSeek V4/V4.1 and HY-V4. The DeepSeek/HY-V4 tests replace the current loader and verify the unsliced global checkpoint reaches it exactly once for each load. AST factory-name scanning and redundant empty/padding tests were removed.No full-model accuracy evaluation, real-checkpoint end-to-end reload, TP>2 or multi-node run was performed. Pipeline parallelism is covered by the skip-path test. Backend-converted sink copies and GPT-OSS expert reload follow-ups are outside this PR; their behavior is unchanged.
Pull Request Checklist
/pr-checklistskill.