[Bugfix] Fix DeepSeek V4 mHC broadcast buffer for dummy load - #51368
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes DeepSeek V4 mHC broadcast-buffer lifecycle so hc_attn_fn_broadcast is always populated for dummy-load initialization and remains pointer-stable across weight refits (important for CUDA graph replay correctness).
Changes:
- Call
finalize_mhc_broadcast_weights()duringDeepseekV4Model.__init__to avoid dummy-load crashes whenload_weights()is skipped. - Update
finalize_mhc_broadcast_weights()to refresh the existing broadcast tensor in-place (vs rebinding) to keep CUDA-graph-captured pointers valid across refits. - Add unit tests asserting (1) correct stream-summed initialization and (2) in-place refresh behavior on refit.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
vllm/models/deepseek_v4/nvidia/model.py |
Ensures broadcast buffer is initialized for dummy load and refreshed in-place on subsequent finalization to keep tensor address stable. |
tests/kernels/test_mhc_kernels.py |
Adds regression tests covering initial broadcast finalization and in-place refresh contract across refit. |
Suppressed comments (1)
tests/kernels/test_mhc_kernels.py:392
- The new DeepSeek V4 broadcast-buffer tests should be skipped when the DeepSeek V4 NVIDIA model import isn't available (see
HAS_DEEPSEEK_V4_NVIDIAfrom the guarded import), otherwise they will error at runtime (e.g., callingDeepseekV4Model.finalize_mhc_broadcast_weights). Add a skip marker to these tests so other kernel tests remain runnable.
def test_deepseek_v4_mhc_broadcast_finalize_sums_hc_streams(monkeypatch):
"""First finalize (run at __init__ so dummy load has a valid buffer)
allocates hc_attn_fn_broadcast as hc_attn_fn summed over hc streams."""
_patch_first_rank_pp_group(monkeypatch)
layer = _make_mhc_decoder_layer(hc_mult=2, hidden_size=8)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2901225 to
16cee4f
Compare
e66aaa2 to
ea9414e
Compare
ea9414e to
9ae42d4
Compare
…mmy load With `--load-format dummy`, `load_weights()` is skipped, so `hc_attn_fn_broadcast` stays `None` and the first decoder layer's 2-D broadcast path fails at `assert self.hc_attn_fn_broadcast is not None`. Fix: define a model-level `process_weights_after_loading()` on `DeepseekV4ForCausalLM`. The loader invokes this hook for every load format — including dummy load — so the broadcast buffer is always populated before the first forward. The hook also finalizes MegaMoE weights, replacing the lazy `finalize_weights()` call in the MegaMoE forward that existed only to cover dummy load. This takes the branch out of the forward hot path and moves the heavy weight repack (which also frees the original params) out of the first forward, so profiling sees steady-state memory. The EPLB-side guard call in `get_expert_mapping` is unchanged. Because the MTP and DSpark drafters load as their own top-level models (registry → `get_model` → loader → PWAL), each gets a matching hook — the removed forward-time call was their only finalize path under dummy load. Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
9ae42d4 to
177101d
Compare
|
✅ @HollowMan6, CI is now available for this PR.
|
|
LGTM, @WoosukKwon could you take a look? |
|
/ci run |
|
✅ Triggered Buildkite CI #84397 for commit |
|
✅ Triggered Buildkite CI #84427 for commit |
Signed-off-by: Hollow Man <hollowman@opensuse.org>
e48860c to
5fd174e
Compare
|
/ci run |
|
✅ Triggered Buildkite CI #84449 for commit |
|
@aoshen02 Now I verified this with verl e2e and found that removing finalize* from load_weights 177101d will actually break the sync, so I reverted the change in the last commit, and it should still be fine: #51368 (comment). After the fix, I checked with verl e2e again by init with dummy weight loader and then reload with the normal weight load, and the training inference mismatch (log probs diff) are consistent and reasonably low at step 1, so I think we are good to go now: rollout_actor_probs_pearson_corr:0.9952459931373596 rollout_probs_diff_mean:0.0063540199771523476 |
…oject#51368) Signed-off-by: Hollow Man <hollowman@opensuse.org> Signed-off-by: Wenhua Cheng <wenhua.cheng@intel.com>
…oject#51368) Signed-off-by: Hollow Man <hollowman@opensuse.org>
…oject#51368) Signed-off-by: Hollow Man <hollowman@opensuse.org> Signed-off-by: Wyett <wyettzeng@gmail.com>
…oject#51368) Signed-off-by: Hollow Man <hollowman@opensuse.org> Signed-off-by: Zhu, Zufang <zufang.zhu@intel.com>
…oject#51368) Signed-off-by: Hollow Man <hollowman@opensuse.org> Signed-off-by: khushali9 <khushali.desai9@gmail.com>
…oject#51368) Signed-off-by: Hollow Man <hollowman@opensuse.org>
…odel Upstream a9f4afb ("[Bugfix] Fix DeepSeek V4 mHC broadcast buffer for dummy load (vllm-project#51368)") added a process_weights_after_loading() method to nvidia/model.py, nvidia/mtp.py and nvidia/dspark.py so that MoE weight finalization also runs when load_weights() is never called (dummy load format, sleep/wake_up reload; see model_loader/utils.py). The merge took jasl's side of the dspark.py conflict, which predates that commit, so the hook landed in model.py and mtp.py but not in dspark.py. Add the hook in the form upstream used (a9f4afb, matching the merged nvidia/mtp.py), calling this flat class's finalize_mega_moe_weights(), and have load_weights() call it as upstream does. finalize_weights() is idempotent, so the extra call is safe.
Purpose
With
--load-format dummy,load_weights()is skipped, sohc_attn_fn_broadcaststaysNoneand the first decoder layer's 2-Dbroadcast path fails at
assert self.hc_attn_fn_broadcast is not None.Fix: define a model-level
process_weights_after_loading()onDeepseekV4ForCausalLM. The loader invokes this hook for every loadformat — including dummy load — so the broadcast buffer is always
populated before the first forward.
The hook also finalizes MegaMoE weights, replacing
the lazy
finalize_weights()call in the MegaMoE forward that existed onlyto cover dummy load. This takes the branch out of the forward hot path and
moves the heavy weight repack (which also frees the original params) out of
the first forward, so profiling sees steady-state memory. The EPLB-side
guard call in
get_expert_mappingis unchanged.Because the MTP and DSpark drafters load as their own top-level models
(registry →
get_model→ loader → PWAL), each gets a matching hook —the removed forward-time call was their only finalize path under dummy
load.
The companion fix for weight refit (in-place
copy_()refresh so thebuffer address stays stable for captured CUDA graphs) is split into a
separate PR.
Test Plan
Purpose
With
--load-format dummy,load_weights()is skipped, sohc_attn_fn_broadcaststaysNoneand the first decoder layer's 2-Dbroadcast path fails at
assert self.hc_attn_fn_broadcast is not None.Fix: define a model-level
process_weights_after_loading()onDeepseekV4ForCausalLM. The loader invokes this hook for every loadformat — including dummy load — so the broadcast buffer is always
populated before the first forward.
Per review (@aoshen02), the hook also finalizes MegaMoE weights, replacing
the lazy
finalize_weights()call in the MegaMoE forward that existed onlyto cover dummy load. This takes the branch out of the forward hot path and
moves the heavy weight repack (which also frees the original params) out of
the first forward, so profiling sees steady-state memory. The EPLB-side
guard call in
get_expert_mappingis unchanged.Because the MTP and DSpark drafters load as their own top-level models
(registry →
get_model→ loader → PWAL), each gets a matching hook —the removed forward-time call was their only finalize path under dummy
load.
The companion fix for weight refit (in-place
copy_()refresh so thebuffer address stays stable for captured CUDA graphs) is split into a
separate PR.
Not a duplicate
Searched open PRs for this area: #50645 guards the broadcast kernel on
DeepGEMM support, #49707 fixes mHC warmup coverage (consumes the buffer,
doesn't populate it), #47807 streamlines warmup on the kernel side. None
touch load-time weight finalization or MegaMoE finalize placement.
Test Plan
Two unit tests added to
tests/models/test_deepseek_v4_mega_moe.py:test_deepseek_v4_pwal_hook_finalizes_mega_moe_and_mhc_broadcast— thetarget model's hook runs both finalizes.
test_deepseek_v4_drafter_pwal_hooks_finalize_mega_moe— the MTP andDSpark hooks delegate to their MegaMoE finalize.
End to end tests on verl side with DSV4 training
Test Result
tests fail — they pin the hooks, not incidentals.
ruff check+ruff format --checkclean.End to end tests on verl side with DSV4 training returned to normal training inference mismatch.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.