Fixed gpt-oss _load_weights_other() parameter position bug#28715
Merged
heheda12345 merged 1 commit intovllm-project:mainfrom Nov 16, 2025
Merged
Fixed gpt-oss _load_weights_other() parameter position bug#28715heheda12345 merged 1 commit intovllm-project:mainfrom
heheda12345 merged 1 commit intovllm-project:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a bug where parameters were passed in the wrong order to the _load_weights_other function. The change is correct and addresses the issue described. I've added one high-severity comment regarding maintainability to prevent similar bugs in the future by making the code more robust.
| ep_rank_start, | ||
| ep_rank_end, |
Contributor
There was a problem hiding this comment.
While this change correctly fixes the argument order, the root cause of the bug is the inconsistent parameter order between _load_weights_other and _load_weights_mxfp4. To improve readability and prevent similar bugs in the future, it's highly recommended to use keyword arguments for this call. This makes the code more explicit and robust against parameter reordering. For example:
return self._load_weights_other(
ep_rank_start=ep_rank_start,
ep_rank_end=ep_rank_end,
heads_per_rank=heads_per_rank,
head_start=head_start,
weights=weights,
stacked_params_mapping=stacked_params_mapping,
)
River12
force-pushed
the
export-D87021773
branch
5 times, most recently
from
eee7487 to
722f941
Compare
River12
force-pushed
the
export-D87021773
branch
from
722f941 to
6aba28e
Compare
…ect#28715) Summary: Signed-off-by: Dezhan Tu <dztu@meta.com> For `_load_weights_other()`, `ep_rank_start` and `ep_rank_end` positions are wrongly placed, leading to the failure of loading expert data in expert parallelism Test Plan: ``` from vllm import LLM llm = LLM("openai/gpt-oss-120b", tensor_parallel_size=2, enable_expert_parallel) output = llm.generate("Hi, vLLM is a") ``` Reviewed By: helunwencser, jackm321 Differential Revision: D87021773
River12
force-pushed
the
export-D87021773
branch
from
6aba28e to
dc52155
Compare
github-project-automation
bot
moved this from To Triage
to Ready
in gpt-oss Issues & Enhancements
github-actions
bot
added
the
ready
github-project-automation
bot
moved this from Ready
to Done
in gpt-oss Issues & Enhancements
devpatelio
pushed a commit
to SumanthRH/vllm
that referenced
this pull request
Nov 29, 2025
…ect#28715) Co-authored-by: Dezhan Tu <dztu@meta.com>
kitaekatt
pushed a commit
to kitaekatt/vllm
that referenced
this pull request
Dec 1, 2025
…ect#28715) Co-authored-by: Dezhan Tu <dztu@meta.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Signed-off-by: Dezhan Tu dezhantu@gmail.com
For
_load_weights_other(),ep_rank_startandep_rank_endpositions are wrongly placed, leading to the failure of loading expert data in expert parallelim. This diff fixed this bug.Test Plan:
Reviewed By: helunwencser, jackm321
Differential Revision: D87021773