Skip to content

[bug] Fix qwen3 weight reloading - #42577

Closed
hao-aaron wants to merge 14 commits into
vllm-project:mainfrom
hao-aaron:weight-sync-fix
Closed

[bug] Fix qwen3 weight reloading#42577
hao-aaron wants to merge 14 commits into
vllm-project:mainfrom
hao-aaron:weight-sync-fix

Conversation

@hao-aaron

@hao-aaron hao-aaron commented May 13, 2026

Copy link
Copy Markdown
Contributor

Fixed a weight loading issue with qwen MoE: HF's Qwen3MoeExperts (transformers ≥ 4.x) packs all experts of an MoE layer into two 3-D tensors per layer (experts.gate_up_proj of shape (E, 2*I, H) and experts.down_proj of shape (E, H, I)), and transformers v5 will make this the universal MoE layout. vLLM's existing per-expert dispatch in Qwen3MoeModel.load_weights doesn't recognize these fused names.

  • vllm/model_executor/layers/fused_moe/layer.py — (a) make_expert_params_mapping appends three HF-fused alias entries (experts.gate_up_proj → w13_weight for w1/w3 with expert_id repurposed as shard_idx, experts.down_proj → w2_weight for w2).
  • vllm/model_executor/models/qwen3_moe.py — Qwen3MoeModel.load_weights plumbs the (augmented) mapping into every layer's FusedMoE and replaces the ~60-line inline expert-dispatch block with a delegation to FusedMoE.load_weights.

Test Plan

Make sure example passes with Qwen/Qwen1.5-MoE-A2.7B-Chat


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

x
Signed-off-by: ahao-anyscale <ahao@anyscale.com>
@hao-aaron
hao-aaron requested a review from njhill as a code owner May 13, 2026 23:06

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

@mergify

mergify Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

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

@mergify mergify Bot added documentation Improvements or additions to documentation v1 bug Something isn't working labels May 13, 2026

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

Copy link
Copy Markdown
Contributor

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 introduces a mechanism to unpack fused MoE weights in the RLHF NCCL example, specifically handling the conversion of fused 3D tensors into the per-expert layout expected by vLLM. Additionally, it updates the GPU worker to ensure the vLLM configuration is correctly set during layerwise weight reloading and updates, which is necessary for certain kernels like FlashInfer CUTLASS MoE. I have no feedback to provide as there were no review comments.

@SumanthRH SumanthRH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

x
Signed-off-by: ahao-anyscale <ahao@anyscale.com>
@mergify mergify Bot added the qwen Related to Qwen models label May 14, 2026
x
Signed-off-by: ahao-anyscale <ahao@anyscale.com>
Comment thread vllm/model_executor/layers/fused_moe/layer.py Outdated
Comment thread vllm/model_executor/models/qwen3_moe.py Outdated
Comment on lines +563 to +569
# Plumb the (augmented) mapping into each FusedMoE so its load_weights
# can run. The mapping is built lazily at load time because base_layer
# detection (for LoRA) needs the fully-constructed model.
for layer in islice(self.layers, self.start_layer, self.end_layer):
if isinstance(layer.mlp, Qwen3MoeSparseMoeBlock):
layer.mlp.experts.expert_mapping = expert_params_mapping
expert_weight_substrings = {w for _, w, _, _ in expert_params_mapping}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we do this when FusedMoE is instantiated rather than patching it in later?

x
Signed-off-by: ahao-anyscale <ahao@anyscale.com>
@hao-aaron hao-aaron changed the title [bug] Fix load weights + layerwise reload requiring config [do not merge][bug] Fix load weights + layerwise reload requiring config May 21, 2026
@hao-aaron hao-aaron changed the title [do not merge][bug] Fix load weights + layerwise reload requiring config [bug] Fix load weights + layerwise reload requiring config May 29, 2026
hao-aaron added 3 commits May 29, 2026 15:19
x
Signed-off-by: hao-aaron <ahao@anyscale.com>
x
Signed-off-by: hao-aaron <ahao@anyscale.com>
@aoshen02 aoshen02 added the ready ONLY add when PR is ready to merge/full CI is needed label May 31, 2026
x
Signed-off-by: hao-aaron <ahao@anyscale.com>
@mergify

mergify Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @hao-aaron.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Jun 1, 2026
hao-aaron added 2 commits June 1, 2026 20:49
x
Signed-off-by: hao-aaron <ahao@anyscale.com>
# Conflicts:
#	vllm/v1/worker/gpu_worker.py

Signed-off-by: hao-aaron <ahao@anyscale.com>
@mergify mergify Bot removed the needs-rebase label Jun 1, 2026
SumanthRH added a commit to NovaSky-AI/SkyRL that referenced this pull request Jun 1, 2026
# H100 CI tests for large MoE models

Introduces an opt-in H100 CI lane that exercises two ~30B-class MoE
models end-to-end:

- **Qwen/Qwen3.5-35B-A3B** — FSDP e2e (`test_policy_local_engines_e2e`),
both colocated and non-colocated with vLLM. Megatron logprob roundtrip
(`test_megatron_models`).
- **nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16** — Megatron logprob
roundtrip (`test_megatron_models`), re-enabled after the original 8-GPU
param was skipped.

Tests are gated by `pytest.mark.h100` (auto-skipped unless `-m h100` is
passed). The new GitHub workflow submits them as an Anyscale staging job
on the `llm-team-h100-4x:1` compute config.

## Test infrastructure

- Register the `h100` marker in
`tests/backends/skyrl_train/gpu/conftest.py` and auto-skip those tests
unless `-m h100` is explicitly passed.
- Add Qwen3.5-35B-A3B parameterizations to
`test_policy_local_engines_e2e` (colocated + non-colocated) and
`test_megatron_models` (TP=4 EP=4).
- Re-enable the Nemotron-3-Nano param on 4 GPUs (TP=1 EP=4) with the
`h100` marker.

## Core fixes uncovered along the way

- **FSDP2 meta-init reordering** (`fsdp_strategy.py`): swap the module
to meta *before* `apply_fsdp2`, so sharded DTensors allocate directly at
their final shard size instead of materializing the full model on rank 0
first. Snapshot/restore non-persistent buffers (e.g.
`RotaryEmbedding.inv_freq`) around the meta swap so they survive the
broadcast.
- **Replace manual broadcast loop with `set_model_state_dict`**
(`fsdp_utils.py`): the ~80-line per-parameter broadcast +
`distribute_tensor` loop is now PyTorch's `set_model_state_dict(...,
StateDictOptions(full_state_dict=True, broadcast_from_rank0=True))`.
- **Remove the FSDP1-era offload+load dance** (`fsdp_utils.py`): the
trick (`offload_fsdp2_model_to_cpu` → `empty_cache` →
`load_fsdp2_model_to_gpu`) was meant to clear reserved-but-unallocated
PyTorch memory. For FSDP2 it's a no-op (`model.to("cpu")` doesn't move
FSDPParam-managed storage during init), then the reload allocates a
second copy — doubling memory. Removed; `set_model_state_dict` already
leaves us with exactly the shard on GPU.
- **NCCL weight-sync receive must be wrapped in
`set_current_vllm_config`** (`new_inference_worker_wrap.py`,
`remote_inference_client.py`, `broadcast_strategy.py`): MoE models
(FlashInfer CUTLASS kernel) read `get_current_vllm_config()` during
`load_weights`, which is only set around `init_device`/`load_model`.
Added a new `update_weights_nccl` worker method that wraps
`weight_transfer_engine.receive_weights` in `set_current_vllm_config`,
and route the broadcast sender through `start_weight_update` +
`update_weights_nccl` + `finish_weight_update` instead of vLLM's native
`/update_weights` endpoint. Same pattern CUDA IPC already used. Tracked
against upstream
[vllm-project/vllm#42577](vllm-project/vllm#42577).
- **Rename `update_weights_chunk` → `update_weights_ipc`** across
`new_inference_worker_wrap.py`, `remote_inference_client.py`, and
`cuda_ipc_strategy.py` so the IPC and NCCL paths have parallel names.
- **Introduce new `inference_only_init` config flag** : disables
optimizer state and allows bf16 trainer to save memory for tests.
- **Large-MoE Megatron config** (`test_megatron_models.py`): for
Qwen3.5-35B and Nemotron-3-Nano, set
`use_precision_aware_optimizer=True` + `optimizer_cpu_offload=True` +
`optimizer_offload_fraction=1.0`. Megatron eagerly materializes the fp32
AdamW state on GPU at init (unlike PyTorch's lazy AdamW), so the
optimizer state alone OOMs without offload.
- **Lower vLLM `gpu_memory_utilization` to 0.5** for these large MoE
models in the Megatron test (`_engine_overrides_for_model`) so the
policy shard + vLLM pool both fit on each H100.
- **`sleep_level=2` in `test_megatron_models`**: the test explicitly
syncs weights, so full sleep (matching the `InferenceEngineState.create`
default and the FSDP e2e test) is the right level. Previously was
hardcoded to 1.

---------

Signed-off-by: ahao-anyscale <ahao@anyscale.com>
Signed-off-by: hao-aaron <ahao@anyscale.com>
Co-authored-by: Sumanth R Hegde <39546518+SumanthRH@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
hmellor added 2 commits June 2, 2026 18:59
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@hmellor

hmellor commented Jun 3, 2026

Copy link
Copy Markdown
Member

The latest change should work with both 2D and 3D weights for Qwen3. It was tested using

# Wait for the vLLM server to be ready before running tests against it.
wait_for_server() {
  until curl -s http://localhost:8000/health > /dev/null; do
    sleep 1
  done
}

# Stop the vLLM server and wait for it to fully exit before continuing, so the
# next server isn't blocked by the old one still holding the port / GPU memory.
stop_server() {
  pkill -f "vllm serve"
  while pgrep -f "vllm serve" > /dev/null; do
    sleep 1
  done
}

# 2D expert weights
## vLLM impl
vllm serve Qwen/Qwen3-30B-A3B-Instruct-2507 --enforce-eager --max-model-len 1024 > server_2d_vllm.log 2>&1 &
wait_for_server
vllm chat -q hello
stop_server
## Transformers impl
vllm serve Qwen/Qwen3-30B-A3B-Instruct-2507 --enforce-eager --max-model-len 1024 --model-impl transformers > server_2d_transformers.log 2>&1 &
wait_for_server
vllm chat -q hello
stop_server
# 3D expert weights
## vLLM impl
vllm serve ../Qwen3-30B-A3B-Instruct-2507 --enforce-eager --max-model-len 1024 > server_3d_vllm.log 2>&1 &
wait_for_server
vllm chat -q hello
stop_server
## Transformers impl
vllm serve ../Qwen3-30B-A3B-Instruct-2507 --enforce-eager --max-model-len 1024 --model-impl transformers > server_3d_transformers.log 2>&1 &
wait_for_server
vllm chat -q hello
stop_server

Where ../Qwen3-30B-A3B-Instruct-2507 is a local version of the checkpoint saved with 3D expert weights using

from transformers import AutoModelForCausalLM, AutoTokenizer


MODEL_ID = "Qwen/Qwen3-30B-A3B-Instruct-2507"
OUTPUT_DIR = "Qwen3-30B-A3B-Instruct-2507"

model = AutoModelForCausalLM.from_pretrained(MODEL_ID)
model.save_pretrained(OUTPUT_DIR, save_original_format=False)

tokenizer = AutoTokenizer.from_pretrained(MODEL_ID)
tokenizer.save_pretrained(OUTPUT_DIR)

The output of the test script should be:

# bash test.sh 

Using model: Qwen/Qwen3-30B-A3B-Instruct-2507
Hello! How can I assist you today? 😊
Using model: Qwen/Qwen3-30B-A3B-Instruct-2507
Hello! How can I assist you today? 😊
Using model: ../Qwen3-30B-A3B-Instruct-2507
Hello! How can I assist you today? 😊
Using model: ../Qwen3-30B-A3B-Instruct-2507
Hello! How can I assist you today? 😊

Comment thread vllm/model_executor/layers/fused_moe/layer.py Outdated
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@aoshen02

aoshen02 commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

In terms of the flashinfer and trtllm assertion, I just make a pr about this #44613. I think it's more elegant, any comments?

Comment on lines +1155 to +1156
expert_name = expert_name.removesuffix(".weight")
qual_name = f"{self.layer_name}.{expert_name}.weight"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the culrit of the failures

@mergify

mergify Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @hao-aaron.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

x
Signed-off-by: haoaaron <ahao@anyscale.com>
@hao-aaron hao-aaron changed the title [bug] Fix load weights + layerwise reload requiring config [bug] Fix layerwise reload requiring config Jun 25, 2026
@hao-aaron hao-aaron changed the title [bug] Fix layerwise reload requiring config [bug] Fix qwen3 weight reloading Jun 25, 2026
@hmellor

hmellor commented Jun 29, 2026

Copy link
Copy Markdown
Member

Weight loading superseded by #47058

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation needs-rebase qwen Related to Qwen models ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants