Skip to content

[Bugfix][RL] Load HY-V4 learnable sinks through weight_loader - #57803

Closed
Ronald1995 wants to merge 2 commits into
vllm-project:mainfrom
Ronald1995:attn-sink-loader-hyv4
Closed

Ronald1995 wants to merge 2 commits into
vllm-project:mainfrom
Ronald1995:attn-sink-loader-hyv4

Conversation

@Ronald1995

@Ronald1995 Ronald1995 commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Fix HY-V4 checkpoint-backed learnable sink loading in NVIDIA target/MTP and AMD target models. The AMD target path was omitted from the original series and is included here.

The runtime parameter already has local TP shape. load_weights takes the rank slice once and passes it to the parameter's current weight_loader; no padded helper or second sharding loader is needed. The AMD and NVIDIA target paths preserve their TP slicing, load the local shard through the current weight_loader, and use the same shared loaded_params path.

Validation

  • This PR changes only three HY-V4 files. ruff check, ruff format --check, and git diff --check pass.

  • Static validation on the integrated final tree: ruff check, ruff format --check, git diff --check, and test tethering pass. The source audit passes 28 tests. The new runtime tests have not been executed, as requested.

AI assistance

AI assistance was used.

@mergify

mergify Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--57803.org.readthedocs.build/en/57803/

@mergify mergify Bot added documentation Improvements or additions to documentation deepseek Related to DeepSeek models DSv4 DSv4.1 Related to DeepSeek-V4.1 models cpu Related to CPU backends dflash bug Something isn't working labels Sep 20, 2026
@Ronald1995
Ronald1995 force-pushed the attn-sink-loader-hyv4 branch from ad8ec0d to 346cae0 Compare September 20, 2026 14:18
Ronald1995 pushed a commit to Ronald1995/vllm that referenced this pull request Sep 20, 2026
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>
@Ronald1995
Ronald1995 force-pushed the attn-sink-loader-hyv4 branch 2 times, most recently from ca0533b to 54b8c9d Compare September 24, 2026 09:31
@Ronald1995
Ronald1995 force-pushed the attn-sink-loader-hyv4 branch from 54b8c9d to a2cefdf Compare September 24, 2026 09:41
Signed-off-by: Ronald1995 <ronaldautomobile@163.com>
@Ronald1995
Ronald1995 force-pushed the attn-sink-loader-hyv4 branch from a2cefdf to cb60c5a Compare September 24, 2026 14:39
HY-V4 checkpoint-backed learnable sinks were written with a bare copy_, so a
live reload left the kernel tensor untouched. All three sites (AMD and NVIDIA
target, NVIDIA MTP) now route the rank slice through the parameter's current
loader via the shared load_padded_attn_sink helper, which is a no-op padding
path here because the HY-V4 sink parameter already has the local head shape.

The target sites also stop falling through into the general weight remapping
code after a sink load, which re-checked a name that is not a checkpoint key
there, and they now record the sink in loaded_params like every other weight.

Signed-off-by: Ronald1995 <ronaldautomobile@163.com>
@Ronald1995

Copy link
Copy Markdown
Collaborator Author

Verification on H20

Verified commit: 69031c5a1480db8eeb8eb1993befdc8a864c56a7 (this PR's head; it covers 3 HY-V4 call sites (amd/nvidia target, nvidia mtp))
Integration commit: c623df9e4bfe1133a3205b94e2d771ba43876ec7 (attn-sink-integration = #57797 + #57798 + #57801 + #57803 + #57804 + #57805 + #57806)

Environment

  • Cluster: H20 Slurm, node vllm-h20-01, 1x NVIDIA H20-3e (--partition=h20 --gres=gpu:h20:1)
  • Python: $HOME/vllm-shared/prebuilt-venv/bin/python (torch 2.13.0+cu130 + compiled vllm from the shared prebuilt baseline)
  • Pure-Python change, so the verified revision was overlaid onto the prebuilt package with make-overlay.sh; nothing was recompiled.

Results

Suite Where Result
test_attention_sink_load_audit.py (from #57806) head node + GPU node 39 passed
test_attn_sink_reload.py (from #57797) head node (CPU) 18 passed, 2 skipped (CUDA-only)
test_attn_sink_reload.py H20 GPU 20 passed
GPU end-to-end sink reload script H20 GPU 8/8 checks passed

The GPU script drives a real layerwise reload (record_metadata_for_reloading -> initialize_layerwise_reload -> load_weights -> finalize_layerwise_reload) on CUDA tensors and asserts the live update lands in the kernel tensor storage with the -inf padding tail re-established and the storage pointer unchanged:

PASS  live update reaches kernel tensor
PASS  live update re-establishes -inf padding
PASS  kernel tensor storage is stable
PASS  rank 1 live update loads its own heads
ALL GPU SINK CHECKS PASSED

Pre-commit

pre-commit run --files <changed files>
pass1 rc=0 dirty=0
pass2 rc=0 dirty=0

No hook rewrote a file. run-precommit.sh skips the pip-compile hooks because the sandbox cannot reach PyPI; no requirements file is touched here.

Not verified

  • No full-model checkpoint was loaded (no matching weights available), so this is loader/contract-level plus a real GPU reload, not a full-model accuracy run.
  • Single GPU; TP>1 was exercised in-process via distinct rank head ranges, not as a multi-process TP run.

What changed in this revision

  • The AMD and NVIDIA target sink branches still fell through into the generic weight-remapping code after loading the sink. They now continue, matching the MTP branch.
  • Both target branches also now add the sink to loaded_params, so callers that validate "all parameters loaded" see the truth. The MTP branch already did.
  • All three sites use the shared load_padded_attn_sink helper instead of a second copy of the inline loader lookup. HY-V4's parameter already has the local head shape, so the helper's padding path is a no-op there.

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

@Ronald1995 Ronald1995 closed this Oct 12, 2026
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Sprint - DFlash Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cpu Related to CPU backends deepseek Related to DeepSeek models dflash documentation Improvements or additions to documentation DSv4 DSv4.1 Related to DeepSeek-V4.1 models

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant