Skip to content

fix: stop shadowing the DSpark shared-experts fusion guard - #39366

Merged
hnyls2002 merged 1 commit into
sgl-project:mainfrom
alphabetc1:fix/dspark-duplicate-fusion-reason
Sep 15, 2026
Merged

hnyls2002 merged 1 commit into
sgl-project:mainfrom
alphabetc1:fix/dspark-duplicate-fusion-reason

Conversation

@alphabetc1

@alphabetc1 alphabetc1 commented Sep 14, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

DeepseekV4ForCausalLMDSpark defines shared_experts_fusion_disable_reason
twice. The second definition wins, so the first one, the only one with the NPU
branch, is dead code:

    @classmethod
    def shared_experts_fusion_disable_reason(cls, hf_config, quant_config):
        if _is_npu:
            return (
                "NPU DSpark ModelSlim weight loading does not support mapping "
                "shared experts into fused expert slots."
            )
        return DeepseekV4ForCausalLM.shared_experts_fusion_disable_reason(...)

    @classmethod                                   # <- this one shadows it
    def shared_experts_fusion_disable_reason(cls, hf_config, quant_config):
        return DeepseekV4ForCausalLM.shared_experts_fusion_disable_reason(...)

The effect is that on NPU, shared-experts fusion is not disabled for the DSpark
draft even though the guard was written to disable it. Per the message that
guard carries, NPU ModelSlim weight loading cannot map shared experts into
fused expert slots, so the draft ends up fusing experts on a path that does not
support it.

CUDA is unaffected: both definitions fall through to the same base
implementation there.

The NPU-aware version arrived in #33676 alongside the plain one from #33312; the
overwrite looks like a merge artifact rather than an intentional revert.

Modifications

Delete the second, shadowing definition. No other change.

Accuracy Tests

Not applicable on CUDA, where both definitions were equivalent and behavior is
byte-identical.

I do not have NPU hardware, so the restored branch is not exercised here. It
restores the behavior #33676 intended; an NPU reviewer confirming that intent
would be the right check before merge.

Speed Tests and Profiling

Not applicable. Dead-code removal with no CUDA behavior change.

Checklist

Found while auditing DSV4 startup; unrelated to that work, so it is on its own.


CI States

Latest PR Test (Base): ❌ Run #34808005605
Latest PR Test (Extra): ❌ Run #34808005393
Latest PR Test (AMD ROCm 10): ❌ Run #34808005509

@alphabetc1 alphabetc1 changed the title [NPU] fix: stop shadowing the DSpark shared-experts fusion guard fix: stop shadowing the DSpark shared-experts fusion guard Sep 15, 2026
@mmangkad

Copy link
Copy Markdown
Contributor

/rerun-test test/registered/unit/models/test_deepseek_v4_shared_expert_fusion.py

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ CI Maintenance Mode is Active

The CI infrastructure is currently under maintenance. All PR CI runs are paused until maintenance is complete. Merging non-CI-fix PRs is prohibited during maintenance mode.

Follow issue #21065 for status updates. Re-run was not dispatched.

@hnyls2002
hnyls2002 merged commit 5298d85 into sgl-project:main Sep 15, 2026
103 of 119 checks passed
@alphabetc1
alphabetc1 deleted the fix/dspark-duplicate-fusion-reason branch September 15, 2026 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants