Conversation
buddywhitman
pushed a commit
to buddywhitman/sglang
that referenced
this pull request
Jul 11, 2026
…detect-and-reject Supersedes the earlier reject-with-error safety net (this branch's prior two commits) with the actual fix, informed by two independent reference implementations that landed after the safety net went in: - tanth47#2 (fork PR): restructures the draft block to gamma+1-wide, separating the anchor slot from the gamma real draft slots. Confirms the right *shape* of the fix, but does so unconditionally for every DSpark checkpoint -- an unverified risk to the already-working DeepSpec-trained checkpoints (jessiewei7's own diagnosis says these work correctly today under the gamma-wide convention; nothing in that PR's test evidence confirms they still do at the new width). - vllm-project/vllm#47093 (merged, validated against real checkpoints: Qwen3-8B, GLM-5.2-DSpark): the same gamma+1-wide restructuring, but gated by a `dspark_bonus_anchor` config flag -- DeepSpec checkpoints keep the exact behavior they had before the flag existed; speculators-format checkpoints get the wider block. This resolves the regression risk in tanth47's version and is the design this PR ports. Changes, mirroring vLLM's `sample_from_anchor`/`dspark_bonus_anchor` split (vllm/v1/worker/gpu/spec_decode/dspark/speculator.py + vllm/transformers_utils/configs/speculators/algos.py): - dspark_config.py: `_resolve_speculators_proposal_gamma` reads the checkpoint's own authoritative draft length from speculators_config.proposal_methods[i].speculative_tokens, instead of deriving it from block_size - 1 and hoping the convention holds universally (ground-truthed against RedHatAI/GLM-5.2-speculator.dspark: block_size=8, speculative_tokens=7 -- these are NOT the same number). DSparkRuntimeConfig now carries speculators_convention through to the worker. - models/dspark.py: removed the earlier reject-ValueError guard -- DSparkDraftMixin no longer needs to know about this at all, since run_markov_block always receives exactly gamma real draft-hidden slots regardless of which convention produced them. - dspark_draft.py: DraftBlockProposer and DsparkDraftSampler both gain a `bonus_anchor` flag (from speculators_convention) and a `draft_width` property (gamma, or gamma+1 when bonus_anchor). Only the draft forward-pass's own block construction changes width; gamma itself, and everything downstream that reads it (verify window sizing, KV commit, accept-length accounting), is completely unaffected -- this is the surgical, low-blast-radius version of the fix, not a codebase-wide gamma redefinition. Added `.contiguous()` after the anchor-hidden-state slice (a real bug I initially missed: PyTorch's `.view()` calls downstream would raise on the resulting non-contiguous tensor). - dspark_worker_v2.py: threads `speculators_convention` from DSparkRuntimeConfig into `self._bonus_anchor`, passed to both the eager (DraftBlockProposer) and CUDA-graph-folded (DsparkDraftSampler) construction sites. Confirmed DSpark has no separate CUDA-graph-capture runner class (unlike the P-EAGLE bug fixed earlier this session) -- _run_forward is the single source of truth for both eager and graph- replay shapes, so there's no second capture-time site to fix. Rebased twice onto hnyls2002's fast-moving tip during this implementation (the branch moved 17 commits between review-response and this commit, including a refactor that extracted the exact gamma-resolution logic this PR touches into a new resolve_runtime_config() function) -- resolved by extending its DSparkRuntimeConfig with the one new field needed rather than duplicating the resolution logic outside it. 8 new unit tests (gamma resolution from speculators_config vs block_size fallback vs explicit default_proposal_method selection; draft_width for both conventions on both DraftBlockProposer and DsparkDraftSampler), 16/16 passing including the 5 pre-existing detection tests. Not hardware-validated end-to-end (no GPU access to the actual checkpoints this fixes) -- the shape/config-resolution logic is unit-tested, but the real accept-length improvement (jessiewei7's reported 1.1 -> ~4.05) needs confirmation against a real speculators-format DSpark checkpoint before this should be treated as fully proven.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack base: DSpark PR branch at 692c5f7. This PR adds the GLM5.2 DSpark correctness baseline: GLM5.2 draft config support, DSpark draft path/default handling, verify length handling, and anchor + draft block layout alignment.\n\nValidation:\n- 27 passed: test_glm52_dspark_config.py, test_dspark_draft_path_default.py, test_dflash_dspark_verify_lengths.py\n\nNote: this is intentionally based on pr-30261-dspark because upstream main does not contain DSpark sgl-project#30261 yet.