Skip to content

fix(qwen3-dflash): post-merge correctness fixes — capture-shape gate, unified readiness, stream - #442

Merged
xiaguan merged 7 commits into
mainfrom
fix/qwen3-dflash-correctness
Jun 23, 2026
Merged

fix(qwen3-dflash): post-merge correctness fixes — capture-shape gate, unified readiness, stream#442
xiaguan merged 7 commits into
mainfrom
fix/qwen3-dflash-correctness

Conversation

@xiaguan

@xiaguan xiaguan commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #436, which merged an earlier snapshot of the DFlash feature. These seven correctness/quality fixes were authored after that snapshot and are not in main — the first two are real correctness defects currently live on main.

Fixes (by severity)

1. Verify CUDA Graph capture-shape bug — silent losslessness break.
The verify piecewise CUDA Graph keyed its captured dense segments by batch_size alone, but total_tokens varies at fixed batch size: a request near its output budget shortens its span (plan.rs token_ids.truncate(remaining)). Captured dense kernels bake their row count, so a graph captured at a short span and replayed at a longer one processes too few rows → trailing requests read stale logits → lossless contract broken under heterogeneous request progress. The bs=1 gate and the homogeneous c8/c16 benches only ever hit the safe (over-compute) direction, so this slipped through. Fixed by gating the captured path on the full batch_size * span shape; any truncated step runs eager. Since each span ≤ block_size, equality holds iff every span is full — capture-shape == replay-shape is now an invariant by construction.

2. Unified path skipped DFlash readiness — feature silently no-opped under mixed load.
The fused Unified step neither marked a request draft-ready (like execute_prefill) nor stripped stale draft context (like execute_decode). A greedy request prefilled through a Unified step never became draft-ready and never recovered. Fixed by routing capture-eligible pending to a dedicated prefill step (build_next_plan's needs_dflash_capture, a deliberate safe superset of the real capture eligibility), and dropping stale context in the Unified decode arm.

3. Stream-override race (graph-capture correctness). copy_hidden_rows_into / copy_hidden_token_range_into launched on the base stream instead of active_cu_stream(ctx). copy_hidden_rows_into runs inside the verify capture, so the copy was not recorded into the graph. Now matches the sibling captured ops.

4. Crash-early guard. Reject DFlash + --decode-overlap at launch — the speculative path never takes the overlap route, so the combination only burned VRAM.

5. Refactor. Split dflash.rs (over the 1k-line limit) into dflash/loading.rs + dflash/reservation.rs; byte-identical move, no behavior change.

6. Test. dflash_concurrent_heterogeneous_is_lossless — concurrent greedy requests at staggered budgets, each checked vs its own plain-greedy baseline. Exercises the bs>1 draft+verify path the bs=1 gate and homogeneous benches never reach.

Verification

  • cargo build --release green; cargo test --release -p openinfer-qwen3-4b --lib → 50 passed.
  • DFlash greedy-lossless gate 4/4 on the local GPU, including the new capture-shape regression and concurrent-heterogeneous tests.
  • Adversarial review pass over all seven changes: no fatal / hard defects.

🤖 Generated with Claude Code

xiaguan and others added 7 commits June 23, 2026 19:58
The piecewise verify graph keyed its captured dense segments by
batch_size alone, but a request near its output budget shortens its
verify span (plan.rs truncates the span to the remaining budget), so
total_tokens varies at a fixed batch_size. The captured dense kernels
bake their row count and run_or_capture is capture-once-replay-forever,
so a graph captured at a short span then replayed at a longer one
processes too few rows and reads stale tail logits — silently breaking
the greedy losslessness contract.

The bs=1 gate and the homogeneous c8/c16 benches both miss it: every
fresh/lockstep request's first verify captures at the full span (the
harmless over-compute direction). The dangerous direction needs
heterogeneous progress, where a bucket is first captured at a truncated
span and later replayed at a full one.

Gate the captured-graph path on total_tokens == batch_size * span; any
truncated step runs eager. Since each span <= block_size, the equality
holds iff every span is full, making capture-shape == replay-shape an
invariant by construction.

Regression test dflash_short_then_long_verify_capture_is_lossless: a
max_tokens=4 request poisons the bucket-bs=1 graph at a truncated span,
then a long request replays it — RED before the gate, GREEN after.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
copy_hidden_rows_into and copy_hidden_token_range_into launched on
ctx.stream.cu_stream() instead of active_cu_stream(ctx). copy_hidden_rows_into
runs inside the verify piecewise CUDA-graph capture path; launching it on the
base stream rather than the captured (override) stream means the copy is not
recorded into the graph. Match the sibling captured ops (add_batch_into,
scaled_add_rows_into) which already use active_cu_stream(ctx).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d step

The fused Unified step (prefill+decode in one forward) neither marked a
request DFlash-ready the way execute_prefill does, nor stripped stale draft
context the way execute_decode does. So a greedy request prefilled through a
Unified step never became draft-ready and never recovered — DFlash silently
no-opped for it under mixed load (no wrong tokens, the feature just quietly
disabled itself).

Route capture-eligible pending (greedy, no LoRA, no logprobs) to a dedicated
prefill step instead of Unified, via build_next_plan's needs_dflash_capture —
mirroring the existing needs_prompt_logprobs precedent — so prefill capture
always runs. The predicate is a deliberate safe superset of the real capture
eligibility: over-routing an ineligible request only costs one fusion, while
under-routing an eligible one into Unified would silently break readiness.

The Unified decode arm now also drops stale draft context for each decoded
request (mirroring execute_decode), keeping the "readiness comes from prefill
capture" invariant closed rather than degrading silently. Also relax the
verify-span budget assert to debug_assert: a continuing active request always
has output budget (resolve finishes it at max_tokens), so it is a proven
invariant, not a runtime condition worth crashing the scheduler thread over.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… launch

The speculative path never takes the unified decode-overlap route, so loading
a DFlash drafter together with --decode-overlap would only spin up overlap
streams that burn VRAM the drafter needs. Reject the combination at launch,
next to the existing DFlash+LoRA guard, instead of silently wasting memory.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…to submodules

dflash.rs was over the 1k-line red-line (1046). Move the byte-identical
from_safetensors_for_target weight-loading method into dflash/loading.rs (a
separate impl DFlashDraftModel block — a descendant module can construct the
parent's private-field struct) and the self-contained DFlashMemoryReservation
into dflash/reservation.rs, re-exported as pub(crate). dflash.rs drops to 835
lines; no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The bs=1 gate and the homogeneous c8/c16 benches never exercise a real batch
of requests at different verify-span lengths — exactly the bs>1 draft+verify
path where a batched-draft indexing bug or a capture-shape mismatch would hide.
Add dflash_concurrent_heterogeneous_is_lossless: submit several greedy requests
at staggered max_tokens at once (they batch in the engine) and check each stays
lossless vs its own plain-greedy baseline via the shared regret check. A
generate_concurrent helper submits all up front, then drains each channel.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…esolved

Distill the review-blockers section: blocker #1 (Unified path skipped DFlash
readiness) and #2 (copy_hidden_* stream race) are now fixed — record the fix
and the new launch-time decode-overlap guard. Note the concurrent heterogeneous
losslessness coverage. gemm_lt_pin_tune (#3) remains the sole open blocker.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@xiaguan
xiaguan merged commit e84e938 into main Jun 23, 2026
1 check passed
@xiaguan
xiaguan deleted the fix/qwen3-dflash-correctness branch June 23, 2026 12:05
wjinxu added a commit to wjinxu/pegainfer that referenced this pull request Jun 24, 2026
…infer-project#442) into n-gram branch

Adopt pegainfer-project#436's method-agnostic speculative core (speculative.rs draft/verify
seam, executor/spec.rs, KV schedule/apply/revert in pool.rs) wholesale,
discarding pegainfer-project#349's now-superseded parallel scaffolding (its own speculative.rs,
scheduler/speculative.rs, executor verify path). The reusable n-gram IP
(ngram.rs prompt-lookup + tests, design doc) is kept unreferenced, to be
re-based onto pegainfer-project#436's proposer seam in follow-up commits.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant