Skip to content

Conversation

@kyuyeunk
Copy link
Contributor

@kyuyeunk kyuyeunk commented Sep 10, 2025

Fixes a bug when the vLLM expects weight loader to return output when using return_success=True

Purpose

Fixes a bug when the vLLM expects weight loader to return output when using return_success=True

Example: qwen3_moe.py

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes a bug in _synced_weight_loader where the return value from the wrapped original_weight_loader was being discarded. The change correctly captures this return value and returns it after the synchronization logic. This is a critical fix as callers, especially in TPU environments where this wrapper is used, may rely on the weight loader's return value, and were previously receiving None. The fix is correct and well-isolated.

Copy link
Collaborator

@yaochengji yaochengji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yaochengji yaochengji added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 10, 2025
@kyuyeunk kyuyeunk force-pushed the fix_synced_weight_loader branch 4 times, most recently from 916bb80 to edbab1b Compare September 11, 2025 02:17
Fixes a bug when the vLLM expects weight loader to return output when using return_success=True

Signed-off-by: Kyuyeun Kim <kyuyeunk@google.com>
@kyuyeunk kyuyeunk force-pushed the fix_synced_weight_loader branch from edbab1b to af0ca8a Compare September 11, 2025 04:43
@DarkLight1337 DarkLight1337 merged commit ed5ae4a into vllm-project:main Sep 11, 2025
38 checks passed
@kyuyeunk kyuyeunk deleted the fix_synced_weight_loader branch September 11, 2025 08:54
skyloevil pushed a commit to skyloevil/vllm that referenced this pull request Sep 13, 2025
Signed-off-by: Kyuyeun Kim <kyuyeunk@google.com>
dsxsteven pushed a commit to dsxsteven/vllm_splitPR that referenced this pull request Sep 15, 2025
Signed-off-by: Kyuyeun Kim <kyuyeunk@google.com>
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: Kyuyeun Kim <kyuyeunk@google.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 10, 2025
Signed-off-by: Kyuyeun Kim <kyuyeunk@google.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
Signed-off-by: Kyuyeun Kim <kyuyeunk@google.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants