Skip to content

[Bugfix][RL] Load MiMo-V2 attention sink bias through weight_loader - #57804

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

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

Conversation

@Ronald1995

@Ronald1995 Ronald1995 commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Fix the MiMo-V2 target model's attention sink bias load. load_weights slices the checkpoint tensor for the local TP rank, then uses the same current parameter loader path as other weights.

The MTP model already follows this contract and is unchanged. This PR changes only mimo_v2.py.

Validation

  • ruff check, ruff format --check, and git diff --check pass.

  • The integrated static audit finds no direct checkpoint sink copy. 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--57804.org.readthedocs.build/en/57804/

@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 labels Sep 20, 2026
@mergify mergify Bot added the bug Something isn't working label Sep 20, 2026
@Ronald1995
Ronald1995 force-pushed the attn-sink-loader-mimo branch from 334846f to 41b7064 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-mimo branch from 41b7064 to d41cfce Compare September 20, 2026 15:00
@Ronald1995
Ronald1995 force-pushed the attn-sink-loader-mimo branch 2 times, most recently from d14853b to f9274f9 Compare September 24, 2026 09:34
@Ronald1995
Ronald1995 force-pushed the attn-sink-loader-mimo branch from f9274f9 to 49e041b Compare September 24, 2026 09:41
Signed-off-by: Ronald1995 <ronaldautomobile@163.com>
@Ronald1995
Ronald1995 force-pushed the attn-sink-loader-mimo branch from 49e041b to a5ab888 Compare September 24, 2026 14:40
The MiMo-V2 target model wrote its head-parallel attention sink bias with a
bare copy_ after slicing the checkpoint tensor by TP rank, so the load was
invisible to layerwise reload and a live update never reached the kernel
tensor. The rank slice is now passed to the parameter's current loader,
matching what mimo_v2_mtp already does.

The sink branch keeps its own narrow-and-load body and continues, so the
generic remap path below it is unchanged for every other weight.

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

Copy link
Copy Markdown
Collaborator Author

Verification on H20

Verified commit: 50ffbf6b74a8b888e72b90ef07247093f80e2bb1 (this PR's head; it covers MiMo-V2 target)
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 earlier revision reindented the whole tail of load_weights to drop an else, which also moved maybe_remap_kv_scale_name in front of the sink handling. That is unrelated to the sink write and changes behavior for a non-sink path. This revision keeps the sink branch self-contained: narrow, load through the current loader, loaded_params.add, continue. The diff is now the sink branch only.

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