Skip to content

[DFlash] Keep grouped-conv taps inside each request's block so NaN/Inf cannot leak across requests - #41736

Merged
kpham-sgl merged 5 commits into
sgl-project:mainfrom
modal-projects:rohan/up/dflash-grouped-conv-nan-leak
Oct 1, 2026
Merged

kpham-sgl merged 5 commits into
sgl-project:mainfrom
modal-projects:rohan/up/dflash-grouped-conv-nan-leak

Conversation

@rodamani

@rodamani rodamani commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

_grouped_conv in models/dflash.py shifts rows across the flattened [bs * block_size] token dimension and masks taps that cross a request's block boundary by multiplying with a 0/1 mask. NaN * 0 and Inf * 0 are NaN, so a non-finite value in request i's last rows leaks into request i+1's first rows, and the DFlash2 draft stack (attention_conv and mlp_conv, prepare and finish, every layer) carries it one request further per layer. One request's bad activations can corrupt its neighbours' drafts.

Modifications

  • Select cross-block taps to exact zeros with torch.where before the multiply. Finite outputs are unchanged apart from the sign of exact zeros.

DSpark

DSparkDraftModel subclasses DFlashDraftModel and reuses its decoder layers and _grouped_conv, so DSpark drafts with conv_kernel_size set get this fix too. Not run on a DSpark checkpoint.

Accuracy Tests

Covered by the existing DFlash CI tests (test_dflash_logits.py, test_basic_sanity_dflash.py, test_basic_sanity_dspark.py).

Speed Tests and Profiling

No hot-path change beyond the fix itself; not separately benchmarked.

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ⏳ Run #36806531824
Latest PR Test (Extra): ❌ Run #36806531535
Latest PR Test (AMD ROCm 10): ⏳ Run #36806532282

…ct#148)

* dflash: keep grouped-conv taps inside each request's block

_grouped_conv shifts rows across the flattened [bs * block_size] token
dimension and masked the taps that cross a block boundary by multiplying
with a 0/1 mask. NaN * 0 and Inf * 0 are NaN, so a non-finite value in
request i's last block rows reached request i+1's first rows, and the
DFlash2 draft stack (attention_conv and mlp_conv, prepare and finish, in
every layer) carried it one request further per layer.

Select the cross-block taps to exact zeros with torch.where before the
multiply. Finite outputs are unchanged apart from the sign of exact
zeros.

* dflash: test the compiled grouped conv at the block boundary

The engine calls _grouped_conv through torch.compile, but every boundary
test forced the eager original with set_stance("force_eager"), so CI
never ran the compiled function on non-finite input.

Run the boundary check and the DFlashGroupedConv prepare/finish check
both eagerly and through the compiled function (inductor on the CPU
runner). With the multiply-by-mask formulation restored in dflash.py,
the compiled checks fail at the same boundary rows as the eager ones
(taps 2 row 7, taps 3 rows 6-7, for NaN, +Inf and -Inf) and in
prepare/finish. The first CPU compile takes about 20 s, so est_time
goes to 30.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@rodamani
rodamani marked this pull request as ready for review September 29, 2026 21:15
@rodamani

Copy link
Copy Markdown
Contributor Author

/rerun-test -c test_dflash_logits.py test_basic_sanity_dflash.py test_basic_sanity_dspark.py

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Results for /rerun-test -c test_dflash_logits.py test_basic_sanity_dflash.py test_basic_sanity_dspark.py:

🚀 ubuntu-latest (1 test): ✅ View workflow run

cd test/ && python3 registered/unit/spec/test_dflash_logits.py

🚀 1-gpu-5090 (1 test): ✅ View workflow run

cd test/ && python3 registered/core/test_basic_sanity_dflash.py

🚀 1-gpu-h100 (1 test): ✅ View workflow run

cd test/ && python3 registered/core/test_basic_sanity_dspark.py

⛔ test/registered/unit/spec/test_dflash_grouped_conv.py: File not found: test/registered/unit/spec/test_dflash_grouped_conv.py

@rodamani

Copy link
Copy Markdown
Contributor Author

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Sep 30, 2026

@kpham-sgl kpham-sgl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address the comments then ping me again

Comment thread test/registered/unit/spec/test_dflash_grouped_conv.py Outdated
Comment thread python/sglang/srt/models/dflash.py Outdated
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@rodamani

rodamani commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

@kpham-sgl comments addressed!

@kpham-sgl
kpham-sgl merged commit b51d4a0 into sgl-project:main Oct 1, 2026
138 of 160 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants