Repository navigation
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
@zyzshishui This PR follows up on the deterministic draft-proposal sampling gap documented in #30822. I would appreciate your review, particularly of the RNG stream separation and EAGLE/NEXTN seed plumbing. |
cf0c551 to
68b6a65
Compare
68b6a65 to
791f50f
Compare
791f50f to
66bae0b
Compare
|
/tag-and-rerun-ci |
|
@ajit283 could you rebase/merge with main? |
|
/rerun-failed-ci |
|
All NV pipelines have passed |
|
@Fridge003 do you know how is in charge of reviewing deterministic features? thanks! |
|
@Qiaolin-Yu Could you help us to review this? Thanks! |
|
@ajit283 could you fix the conflicts? thanks! |
|
[by Codex] The required PR Test Base failure is caused by this PR, not an intermittent CI issue. test_chain_read_ratchet.py flags two new reads of the resolved enable_deterministic_inference setting through borrowed server_args objects:
Please read the resolved config bag instead, using get_exec().deterministic.enable_deterministic_inference (and import get_exec from sglang.srt.runtime_context) at both sites. Failed job: https://github.com/sgl-project/sglang/actions/runs/33074073313/job/98523815165 |
|
[by Codex] @ajit283, the relevant CI failure appears to be caused by this PR: https://github.com/sgl-project/sglang/actions/runs/33450499018/job/99686771962. The PR-added |
nvpohanh
left a comment
There was a problem hiding this comment.
[by Claude Code] Review of the deterministic draft-proposal sampling change. Inline comments below; each is labeled with severity and confidence.
|
@ajit283 could you address the comments? thanks |
nvpohanh
left a comment
There was a problem hiding this comment.
[by Claude Code] Re-review at f004e95. The latest commit fixes three of my four earlier threads (draft-extend position key, step/seed aliasing, and the double draft_probs clone), so I am resolving those threads. The get_spec() thread on eagle_draft_cuda_graph_runner.py line 209 is still open. One new inline comment below.
…unner Gate the draft CUDA graph sampling_seed buffer on the resolved get_spec().speculative_use_rejection_sampling, matching the draft_probs buffer above it, instead of the raw server_args field. Drop test_murmur_hash32_cpu_distinguishes_draft_step_column_domains: it only checks murmur_hash32 and never calls fast_sample, so it cannot catch a regression in the draft-step column offset. The GPU test test_seed_position_and_draft_step_select_separate_streams covers it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/tag-and-rerun-ci |
|
[by Claude Code] @ajit283, the relevant CI failure appears to be caused by this PR: https://github.com/sgl-project/sglang/actions/runs/37007979535/job/110841646756. The PR-added |
sgl-project#41175 made `_draft_extend_for_decode` read `batch_result.prepared_draft_extend_inputs`, so the mocked batch result in test_eagle_draft_sampling.py raised AttributeError after merging main. Set it to None so the test takes the CPU fallback path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
[by Claude Code] @ajit283, following up on my comment above: I pushed c1b3d4c to your branch. It adds |
|
/rerun-failed-ci |
Resolve the sample_draft_proposal conflict by keeping main's sampling_softmax while passing the PR's seeded draft-sampling arguments to fast_sample. main switched fast_sample's single-sample argmax to the FP32-only row_argmax kernel; keep torch argmax for the seeded path's float64 scores. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/rerun-failed-ci |
1 similar comment
|
/rerun-failed-ci |
|
/rerun-failed-ci |
Picks up the latest main for a fresh CI run after a flaky DSV4-Flash H200 accept-length failure (https://github.com/sgl-project/sglang/actions/runs/37251609204/job/111751683459). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
[by Claude Code] @ajit283, the relevant CI failure appears to be caused by this PR: https://github.com/sgl-project/sglang/actions/runs/37420455068/job/112128680788. After merging the latest |
Block verification forces rejection sampling, which this PR makes deterministic via seeded draft and verify draws, so --enable-deterministic-inference is no longer rejected on CUDA. Update the test from sgl-project#42297 accordingly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
[by Claude Code] Follow-up: I pushed f0e205e to address the failure above with a test-only change. Block verification reuses this PR's seeded draft and verify draws, so |
|
[by Claude Code] The relevant CI failure (https://github.com/sgl-project/sglang/actions/runs/37431477801/job/112163452489) is not caused by this PR. It is a known breakage on |
Picks up sgl-project#42750, which fixes the unrelated CI failure https://github.com/sgl-project/sglang/actions/runs/37431477801/job/112163452489 (test_dsa_metadata_replay TARGET_VERIFY mismatch). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/rerun-failed-ci |
|
/rerun-failed-ci |
1 similar comment
|
/rerun-failed-ci |
|
[by Claude Code] All NV pipelines have passed. |
|
@Qiaolin-Yu could you help to review this? Thanks! |
Motivation
Classic EAGLE/NEXTN rejection sampling draws each draft proposal with
fast_sample(). Although target sampling and verify-side rejection coins support request-local seeds, this proposal draw used the global PyTorch RNG (exponential_()), so--enable-deterministic-inferencestill changed outputs with batch shape and CUDA graph replay.This is the draft-worker follow-up documented in #30822. Before this change, Qwen3.5-9B NEXTN rejection sampling produced 12 unique outputs in 12 changing-batch trials with a fixed sampling seed.
Modifications
fast_sample()keyed by:Accuracy Tests
Regular EAGLE/NEXTN
Qwen3.5-9B, TP2, NEXTN, rejection sampling, temperature 1.0, sampling seed 42:
The deterministic harness also passed changing batch sizes, prefix batching, bitwise logprob comparison, and radix-cache consistency.
Post-rebase validation on B200:
The unit suite includes 100,000 distinct seeded draws from
[0.1, 0.2, 0.7]and verifies that observed frequencies are within 0.01 absolute tolerance.Multi-layer EAGLE
stepfun-ai/Step-3.5-Flash, TP8 on 8x B200, FA4, three MTP layers, single-CUDA-graph draft extend:Speed Tests and Profiling
B200, vocabulary size 163,840, CUDA-graph replay:
Peak allocated memory for batch 128 and vocabulary 163,840:
The deterministic mode is opt-in. A fused hash/exponential/argmax kernel can be considered separately if end-to-end profiling shows that the materialized hash and score matrices materially affect serving throughput.
Checklist
git diff --check.CI States
Latest PR Test (Base): ✅ Run #37454576286
Latest PR Test (Extra): ❌ Run #37454575883
Latest PR Test (AMD ROCm 10): ❌ Run #37454576246