Conversation
|
This pull request has merge conflicts that must be resolved before it can be |
b68c9ca to
5c01e3a
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces extensive updates to vLLM, including automated Docker image publishing, support for input clamping in SwiGLU activations, and a fused hc_head kernel implemented with TileLang. Significant optimizations are added for DeepSeek-V4, such as multi-stream GEMM overlap and expert-dtype-aware MoE dispatch. Additionally, the V1 engine's KV cache management is refined to better handle block recycling for sliding window and chunked-local attention. Review feedback correctly identified critical bugs in the DeepSeek-V4 attention layer where ReplicatedLinear outputs were not unpacked from their return tuples before being reshaped, which would cause attribute errors at runtime.
I am having trouble creating individual review comments. Click here to see my feedback.
vllm/model_executor/layers/deepseek_v4_attention.py (427)
The ReplicatedLinear layer returns a tuple of (output, bias). Attempting to call .view() directly on the return value will raise an AttributeError because tuple does not have a view method. You must unpack the output first.
q, _ = self.wq_b(qr)
q = q.view(-1, self.n_local_heads, self.head_dim)
vllm/model_executor/layers/deepseek_v4_attention.py (453)
Similar to the issue above, self.wq_b(qr) returns a tuple (output, bias). You need to unpack the output before calling .view().
q, _ = self.wq_b(qr)
q = q.view(-1, self.n_local_heads, self.head_dim)
vllm/model_executor/layers/deepseek_v4_attention.py (466)
The ReplicatedLinear layer returns a tuple. Unpack the output before applying the view transformation.
q, _ = self.wq_b(qr)
q = q.view(-1, self.n_local_heads, self.head_dim)
|
Confirmed the bug and verified this fix end-to-end on DGX Spark (GB10, sm_121, CUDA 13.0). Wanted to share a regression test since the PR doesn't currently include one — happy to Repro on stock
|
…m-project#42601) PR vllm-project#42601 fixes a NaN bug in `make_nvfp4_moe_quant_config` where checkpoints containing dead experts (uncalibrated, scale == 0.0) produce `1.0 / 0.0 == inf` global activation scales, which the FP4 quantization kernel then turns into NaN tokens. This change adds a regression test exercising the exact buggy path (`a13_scale` / `a2_scale` containing zeros) across all five backends that go through the inversion (VLLM_CUTLASS and the four FLASHINFER_* variants); MARLIN and EMULATION take separate branches and are not affected. The test is platform-independent (no CUDA kernel calls) so it runs in the standard CI matrix and provides fast regression coverage if anyone later removes the clamp. Validated locally on DGX Spark (GB10, sm_121, CUDA 13.0): - On `vllm==0.20.2` (pre-fix): 5 of 6 fail with `a1_gscale = inf` / `a2_gscale = inf` exactly at the dead-expert indices; the `_healthy_scales_unchanged` sanity case passes (clamp is a no-op for normal scales). - With the vllm-project#42601 patch applied: all 6 pass in ~3 s. Depends on vllm-project#42601. This PR will be marked ready once that lands; until then this branch's CI will surface the pre-fix failure as documented. Signed-off-by: Henry Lu <Henry.Lu@aiunion.com.tw>
|
Confirmed both bug and fix on SM120. |
…xperts Some NVFP4 checkpoints (e.g. Qwen3.5-122B-A10B-NVFP4) contain near-zero or denormal activation scales for dead MoE experts. Inverting these scales (1/scale) produces Inf, which propagates as NaN through the FP4 quantization kernel and corrupts the entire batch. Fix: clamp a1_gscale and a2_gscale inputs from below at torch.finfo(float32).tiny (~1.18e-38) before inversion in make_nvfp4_moe_quant_config(). This is mathematically safe — dead experts produce zero-weighted outputs regardless of their scale. Signed-off-by: Pavel Zakharov <pavel.zakharov@gmail.com>
b2682da to
942c353
Compare
|
Friendly ping — this has been open since May and still shows mergeable against main. The fix has since been battle-tested further: it's been running in production on a 2× DGX Spark (GB10) TP=2 cluster serving DeepSeek-V4-Flash-0731 (NVFP4 experts) as part of the stack described in #52499–#52503; without the clamp, dead experts produce NaN activations on this checkpoint. Happy to rebase if maintainers prefer a fresh CI run. |
Refer to #45320 / #54444, they have a better way to deal with such situation. And input_scale will be used in: if the input_scale is zero, the original path will make a_gscale inf and weight_scale_2 0, the result is inf * 0 = NaN which can be observed by the numeric issue. But this PR makes the result become zero, how can we make sure it's a good number? |
|
Thanks for the review. I agree that making the reciprocal finite does not establish output correctness, and my numerical-safety claim was too strong. I no longer use the affected model and cannot commit to the additional validation needed, so I’m closing this workaround. The load-time validation approach in #45320 / #54444 is a better direction for missing or invalid scales. |
Summary
Some NVFP4 checkpoints (e.g.
txn545/Qwen3.5-122B-A10B-NVFP4) have near-zero or denormalfloat32activation scales for dead MoE experts. Inmake_nvfp4_moe_quant_config(), these are inverted directly (1.0 / a13_scale,1.0 / a2_scale), producingInf, which the FP4 quantization kernel propagates asNaN— corrupting the entire output batch.Fix: clamp both activation global scales from below at
torch.finfo(torch.float32).tiny(~1.18e-38) before inversion. Dead experts contribute zero-weighted outputs regardless of their scale value, so this clamp is numerically safe.oracle/nvfp4.py.Test plan
txn545/Qwen3.5-122B-A10B-NVFP4on DGX Spark (SM12.1, CUDA 13.1) with vLLM v0.20.2 + this patch