[Bug] R1 Accuracy: Fix routed_scaling_factor Double Mul Issue

[yewentao256]

Purpose

Fixes #24118 that is introduced by #23123

Test

lm_eval --model local-completions --model_args "base_url=http://127.0.0.1:9256/v1/completions,model=deepseek-ai/DeepSeek-R1,num_concurrent=256" --tasks gsm8k

Origin

|Tasks|Version|     Filter     |n-shot|  Metric   |   |Value |   |Stderr|
|-----|------:|----------------|-----:|-----------|---|-----:|---|-----:|
|gsm8k|      3|flexible-extract|     5|exact_match|↑  |0.6626|±  |0.0130|
|     |       |strict-match    |     5|exact_match|↑  |0.5201|±  |0.0138|

Now

|Tasks|Version|     Filter     |n-shot|  Metric   |   |Value |   |Stderr|
|-----|------:|----------------|-----:|-----------|---|-----:|---|-----:|
|gsm8k|      3|flexible-extract|     5|exact_match|↑  |0.9530|±  |0.0058|
|     |       |strict-match    |     5|exact_match|↑  |0.9522|±  |0.0059|

[gemini-code-assist]

Code Review

This pull request correctly fixes a bug in the DeepseekV2MoE forward pass where routed_scaling_factor was being applied twice for non-fp16 data types. The self.experts layer (FusedMoE) already applies this scaling factor internally. By removing the redundant multiplication, the code is now more correct. I have also added a review comment highlighting a potential related scaling issue in the handling of shared_output that may require further investigation.

[tlrmchlsmth]

This is handled here right?

vllm/vllm/model_executor/layers/fused_moe/fused_moe.py

Lines 1014 to 1015 in 98aee61

	if routed_scaling_factor != 1.0:
	topk_weights = topk_weights * routed_scaling_factor

Why does main not apply the routed_scaling_factor in the fp16 case? Is there an accuracy issue we need to handle?

[sarckk]

looks like it's to prevent fp16 overflow issue for DeepSeek V2: #13232

by multiplying by routed scaling factor in all cases, #23123 might have introduced an accuracy regression for V2 as well, we should check this.

[yewentao256]

Yes, so I changed another way to fix since deepseek v2 has a lot of logic for routed_scaling_factor outside

[sarckk]

thanks for the fix

I think we also need this fix for dots1 and glm4_moe? both use use_grouped_topk=True so they should be affected by the double mul issue as well

vllm/vllm/model_executor/models/dots1.py

Line 163 in 98aee61

router_logits=router_logits) * self.routed_scaling_factor

vllm/vllm/model_executor/models/glm4_moe.py

Line 189 in 98aee61

router_logits=router_logits) * self.routed_scaling_factor

[yewentao256]

thanks for the fix

I think we also need this fix for dots1 and glm4_moe? both use use_grouped_topk=True so they should be affected by the double mul issue as well

vllm/vllm/model_executor/models/dots1.py

Line 163 in 98aee61

router_logits=router_logits) * self.routed_scaling_factor

vllm/vllm/model_executor/models/glm4_moe.py

Line 189 in 98aee61

router_logits=router_logits) * self.routed_scaling_factor

@sarckk Nice find! Feel free to have a pr towards this.

[houseroad]

Thanks for the fix.

[facebook-github-bot]

@sarckk has imported this pull request. If you are a Meta employee, you can view this in D81519055.

[WoosukKwon]

@yewentao256 Thanks for the fix. BTW, can you please add a test in our CI? I think we've seen similar accuracy issues 5+ times on DeepSeek...

[houseroad]

Besiding running full deepseek model, is it possible to run a smaller model to detect such issue, like DeepSeek v2, etc.