Skip to content

Fix chunked prefix cache for nvfp4#10180

Merged
zhyncs merged 19 commits intosgl-project:mainfrom
wenscarl:chunked_prefix_fix
Sep 12, 2025
Merged

Fix chunked prefix cache for nvfp4#10180
zhyncs merged 19 commits intosgl-project:mainfrom
wenscarl:chunked_prefix_fix

Conversation

@wenscarl
Copy link
Copy Markdown
Collaborator

@wenscarl wenscarl commented Sep 8, 2025 •
edited
Loading

co-authored by @elfiegg

Motivation

To address issue 9806

Modifications

Accuracy Tests

server.sh
python3 -m sglang.launch_server \
  --model-path nvidia/DeepSeek-R1-0528-FP4 \
  --trust-remote-code \
  --attention-backend trtllm_mla \ or flashinfer \
  --disable-radix-cache \
  --max-running-requests 256 \
  --chunked-prefill-size 2048 \
  --mem-fraction-static 0.89 \
  --max-prefill-tokens 32768 \
  --disable-cuda-graph \
  --tp 8 \
  --dp 8 \
  --enable-dp-attention \
  --quantization modelopt_fp4 

client.sh
python3 benchmark/gsm8k/bench_sglang.py --num-shots 8 --num-questions 1310 --parallel 1310

flashinfer attention backend
before:
Accuracy: 0.510
Invalid: 0.480

after:
Accuracy: 0.948
Invalid: 0.000


trtllm_mla attention backend
before:
Accuracy: 0.421
Invalid: 0.429

after:
Accuracy: 0.952
Invalid: 0.001

Benchmarking and Profiling

Checklist

Fridge003
Fridge003 reviewed Sep 9, 2025
View reviewed changes
Comment thread python/sglang/srt/models/deepseek_v2.py Outdated
wenscarl
wenscarl commented Sep 9, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator Author

@wenscarl wenscarl left a comment

Choose a reason for hiding this comment

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

Work around cutlass kernel for chunked prefix

@wenscarl wenscarl marked this pull request as ready for review September 9, 2025 19:04
@wenscarl wenscarl requested a review from Fridge003 September 9, 2025 19:04
@wenscarl
temp fix
5fcba1d 
Revert "[NVIDIA] disable chunked prefix cache when dp and blackwell is used (sgl-project#9861)"

This reverts commit 90dfe3d.
@wenscarl
Work around cutlass kernel for chunked prefix
5074cbd
zhyncs
zhyncs reviewed Sep 9, 2025
View reviewed changes
Comment thread python/sglang/srt/layers/attention/flashinfer_mla_backend.py Outdated
@Fridge003
Copy link
Copy Markdown
Collaborator

Fridge003 commented Sep 9, 2025

@wenscarl Will the accuracy of dpsk-r1 nvfp4 be back to normal after changing flashinfer kernel to fa2 version?

@wenscarl
Copy link
Copy Markdown
Collaborator Author

wenscarl commented Sep 9, 2025

@wenscarl Will the accuracy of dpsk-r1 nvfp4 be back to normal after changing flashinfer kernel to fa2 version?
Yes. It's in the description.

@wenscarl wenscarl requested a review from zhyncs September 10, 2025 02:32
Fridge003
Fridge003 approved these changes Sep 10, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Fridge003 Fridge003 left a comment

Choose a reason for hiding this comment

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

LGTM

@Fridge003 Fridge003 mentioned this pull request Sep 10, 2025
Closed
4 tasks
yzh119
yzh119 reviewed Sep 10, 2025
View reviewed changes
Comment thread python/sglang/srt/layers/attention/flashinfer_mla_backend.py Outdated
@elfiegg
Copy link
Copy Markdown
Collaborator

elfiegg commented Sep 10, 2025

can we route the logic based on quantization temporarily ? FP8 model works well with cutlass backend, and using FA2 the perf would drop to 1/2.

Fridge003
Fridge003 reviewed Sep 10, 2025
View reviewed changes
Comment thread python/sglang/srt/models/deepseek_v2.py Outdated
elfiegg
elfiegg reviewed Sep 10, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@elfiegg elfiegg left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix!

Comment thread python/sglang/srt/models/deepseek_v2.py
elfiegg
elfiegg reviewed Sep 10, 2025
View reviewed changes
Comment thread python/sglang/srt/models/deepseek_v2.py Outdated
elfiegg
elfiegg reviewed Sep 10, 2025
View reviewed changes
Comment thread python/sglang/srt/models/deepseek_v2.py Outdated
@kushanam
Copy link
Copy Markdown
Collaborator

kushanam commented Sep 11, 2025

This PR already includes the changes from #10178

@Fridge003 Fridge003 mentioned this pull request Sep 11, 2025
Closed
Fridge003
Fridge003 reviewed Sep 11, 2025
View reviewed changes
Comment thread python/sglang/srt/models/deepseek_v2.py Outdated
Comment thread python/sglang/srt/models/deepseek_v2.py Outdated
Comment thread python/sglang/srt/models/deepseek_v2.py Outdated
@Fridge003 Fridge003 added the ready-to-merge The PR is ready to merge after the CI is green. label Sep 11, 2025
@Fridge003 Fridge003 self-assigned this Sep 11, 2025
@Fridge003
Copy link
Copy Markdown
Collaborator

Fridge003 commented Sep 11, 2025

@wenscarl Please fix lint with

pre-commit run --all-files

@fzyzcjy
Copy link
Copy Markdown
Collaborator

fzyzcjy commented Sep 12, 2025

looking forward to this

@zhyncs zhyncs merged commit 36acd2f into sgl-project:main Sep 12, 2025
176 of 193 checks passed
@rainj-me rainj-me mentioned this pull request Sep 12, 2025
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merrymercy merrymercy Awaiting requested review from merrymercy merrymercy is a code owner

@Ying1123 Ying1123 Awaiting requested review from Ying1123 Ying1123 is a code owner

@hnyls2002 hnyls2002 Awaiting requested review from hnyls2002 hnyls2002 is a code owner

@ispobock ispobock Awaiting requested review from ispobock ispobock is a code owner

@HaiShaw HaiShaw Awaiting requested review from HaiShaw HaiShaw is a code owner

@ch-wan ch-wan Awaiting requested review from ch-wan ch-wan is a code owner

@BBuf BBuf Awaiting requested review from BBuf

@kushanam kushanam Awaiting requested review from kushanam

@Edwardf0t1 Edwardf0t1 Awaiting requested review from Edwardf0t1

@zhyncs zhyncs Awaiting requested review from zhyncs

@Fridge003 Fridge003 Awaiting requested review from Fridge003 Fridge003 is a code owner

2 more reviewers

@yzh119 yzh119 yzh119 left review comments

@elfiegg elfiegg elfiegg left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@yzh119 yzh119

@Fridge003 Fridge003

Labels

high priority ready-to-merge The PR is ready to merge after the CI is green.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@wenscarl @Fridge003 @elfiegg @zhyncs @kushanam @fzyzcjy @yzh119