Repository navigation
Phase 5b: speculative decoding, RoPE inv_freq root-cause fix, per-phase findings folders - #6
Merged
Merged
Conversation
Real L40 session (RunPod Secure Cloud, US-KS-2, $0.6833 of $10 cap): both drafters (draft-model, prompt-lookup) pass exact-match correctness against a plain-greedy baseline on the real 16B target under the int8 quantized kernel. Draft-model: 91.9% acceptance, +21.8% throughput at k=4. Prompt-lookup: 22.1% acceptance, +76.9% throughput at k=4 (lower acceptance but near-zero overhead wins on raw speed). Memory checkpoints confirm the design's budget rationale: quantized target alone allocates 16.75GB, target+draft together 29.62GB, comfortably under the ~44.4GB L40. Five distinct DeepSeek remote-code breaks found and fixed live against transformers==5.17.0 (is_torch_fx_available, get_usable_length, from_legacy_cache, and to_legacy_cache all removed/changed; rope_scaling auto-populated into a dict missing the old "type" key). Deliberately did not downgrade transformers this time -- Phase 0/1/5a's own established downgrade fix would have silently broken this phase's own Cache.crop() negative-number semantics that Tasks 2-3 depend on -- so kept the pinned version and monkeypatched the specific broken symbols instead. All folded into the runbook for future phases. The k-sweep ran the full 4-prompt suite at every k rather than just the originally-planned repetition-heavy prompt alone, since the CLI has no per-prompt selection flag and adding one mid-session wasn't worth the risk for a pure cost optimization; noted in the runbook.
…o-end draft-model test; withdraw unverified GPU throughput numbers Final whole-branch review found the baseline oracle (--drafter none) was generated by the same shared verify/accept/reject loop the correctness gates were meant to check, making a real degenerate GPU session output (all 4 prompts collapsing to a single repeated token) pass as a "correctness match" against itself. Adds a genuinely independent plain-greedy oracle, runtime cache-lag assertions, and a real end-to-end CPU test of DraftModelDrafter inside the loop -- none of which existed before. Withdraws the GPU session's throughput and correctness-gate claims from the findings doc and STATUS.md pending a root-cause investigation (out of scope here, needs a new GPU session).
…o the runbook Final whole-branch review found the prior GPU session's baseline degenerated (repeating one token id across all 4 prompts) and both correctness gates passed only because they matched that same broken output -- a vacuous pass. Fixes two real gaps for the next session: a stock-vs-quantized investigation step to localize the cause before spending more money, and an explicit baseline-plausibility check (decode and eyeball the tokens) before trusting any token_match result. Also closes finding I5: the k-sweep now checks --compare-generated-tokens at every k, not just k=4.
…5.17.0 Root-caused the C1 degenerate-baseline finding on a second GPU session: DeepSeek's remote code collapses attention_mask to None for an unpadded single-sequence input, then eager attention builds a causal 4D mask via the deprecated transformers.modeling_attn_mask_utils path. That path produces all-NaN logits under transformers==5.17.0 (the version pinned for Cache.crop()'s negative-argument semantics), independent of dtype (reproduced in both bf16 and fp16) and independent of quantization (reproduced on the stock, unpatched model). torch.argmax on an all-NaN row returns index 0 by first-occurrence tie-break, which is exactly the degenerate all-token-0 output the final whole-branch review flagged. Forcing attn_implementation="sdpa" on both the target and draft model loads sidesteps the deprecated mask construction entirely (sdpa relies on its own is_causal fast path when no mask is passed) and produces real, varied, plausible logits with no NaN. Verified live against the real 16B model on a rented L40: eager reproduces the NaN exactly, sdpa does not. load_model() gains an optional attn_implementation parameter, forwarded to from_pretrained only when given, so every other call site's existing behavior is unchanged.
…() call Found live on the second GPU session, immediately after the sdpa fix made real (non-degenerate) generation happen for the first time: run_speculative_bench.py builds one DraftModelDrafter and reuses it across 20 separate speculative_generate() calls (4 prompts x 5 repetitions), but DraftModelDrafter.propose() only feeds the whole sequence when its own past_key_values is None -- true only for the very first call ever made on that instance. Every later call, including ones for a completely different prompt, was treated as a continuation of whatever sequence came before it: the drafter's cache grew across all 20 runs instead of restarting per prompt, eventually exhausting the GPU's memory (the run that hit this went silent and the SSH connection dropped, consistent with a Linux OOM-kill). Session 1 never surfaced this: every prompt's baseline was already degenerate (all-token-0, the NaN bug fixed in the prior commit), so a corrupted drafter cache made no visible difference to an already-broken result. Adds reset() to the Drafter protocol (a no-op for stateless PromptLookupDrafter, clears past_key_values for DraftModelDrafter) and calls it at the top of run_speculative_rounds, alongside the target's own fresh past_key_values = None -- fixing this at the shared loop's level so any future caller that reuses a drafter across calls is correct by construction, not by caller discipline.
AutoModelForCausalLM/AutoTokenizer accessed via harness_module.X are an implicit reexport, which --strict flags. Import them directly from transformers instead -- same class objects at runtime, so monkeypatching their from_pretrained still reaches harness.load_model's own calls.
…ot cause of the degenerate baseline) Root-caused via layer-by-layer NaN isolation on a real GPU session: NaN first appears in layer 0's self_attn, before the MoE FFN is ever touched. Tracing deeper (per-submodule hooks on q_proj/k_proj/v_proj/ o_proj/rotary_emb) found it: DeepseekRotaryEmbedding's inv_freq buffer --computed fresh in __init__ as 1/base**(i/dim), marked persistent=False because it's not meant to be part of a checkpoint's state_dict -- is left as uninitialized memory by transformers==5.17.0's model-loading path instead of holding that computed value. Confirmed the corruption is present immediately after AutoModelForCausalLM. from_pretrained returns, before any .to(device) call -- not a GPU/CUDA numerics issue despite only reliably producing NaN once a forward pass runs on GPU (this explains why a CPU forward pass reading the same garbage inv_freq produced real, non-NaN, wrong-content output instead, and why the bug reproduced identically across two different GPU architectures, hosts, and every attn_implementation tested including plain "math" SDPA). This supersedes this branch's earlier "force sdpa" fix. That change is kept (sdpa is a reasonable choice on its own merits), but the comments claiming it fixed the eager-masking path have been corrected: that distinction doesn't hold up under this fully identified root cause -- the actual bug reproduced under every attn_implementation tested, sdpa included. fix_rope_inv_freq() (dispatch.kernels.integration, alongside the other after-load DeepSeek remote-code repairs) discovers rotary-embedding modules by duck typing (inv_freq/dim/base attributes, matching this file's existing moe_infer discovery pattern) and recomputes each one's inv_freq correctly, resetting its cached cos/sin so the next forward call rebuilds them from the corrected values. Wired into run_speculative_bench.py right after loading both the target and draft models. Verified live on a real GPU: after the fix, inv_freq matches the expected closed-form sequence exactly, and greedy-decoding "The quick brown fox jumps over the lazy dog." produces a real, coherent continuation instead of 64 repetitions of token 0.
fix_rope_inv_freq (commit c5b59df) fixed the actual bug; this commit records the real GPU re-run it made possible: a non-degenerate baseline, both correctness gates, and the full k-sweep checked at every point. Supersedes the WITHDRAWN framing in the prior findings doc and STATUS.md. The one real gate divergence (prompt-lookup) and the k-sweep's wider pattern (prompt_000 diverging in most configs, including draft-model at k=1/k=2/k=8) are root-caused to a genuine near-tied logit position under the int8-quantized kernel's real floating-point precision, confirmed by two independent GPU probes -- not a defect in dispatch.speculative, which was re-derived by hand and found correct. Also documents a second, unrelated infrastructure finding hit while retrieving this session's evidence: the pod's repo clone lived outside its persistent mount and did not survive a stop/start cycle. The 11 results.json files here are exact reconstructions from this session's own already-captured process output; the corresponding raw generated-tokens.json files could not be recovered this way and are not included. Total real GPU cost across both Phase 5b sessions: $12.26 of the $20 cap. Flags, but does not audit, whether Phase 5a's own correctness claim should be re-examined given this session's concrete evidence of near-tie sensitivity in the same quantized kernel.
Final-review finding: fix_rope_inv_freq was wired into exactly one of several DeepSeek-loading entrypoints (scripts/run_speculative_bench.py), while run_baseline.py -- which produces the reference logits other correctness gates compare against -- and every scripts/gpu/*.py script had no protection against the same bug, despite the pinned transformers>=5.17.0 guaranteeing it. Moves the fix into dispatch.benchmark.harness.load_model itself so every caller gets it by construction; idempotent and harmless on a model whose rotary embedding doesn't match the duck-typed shape. run_speculative_bench.py keeps one explicit call on the target only, to capture the count for its results JSON and guard against silent regression (0 fixed now refuses the run, ordered after the existing moe_layers_patched guard so a model with neither shape still reports the pre-existing error). Also records attn_implementation in the results JSON (previously only in the prose findings doc). Test-quality fixes: the inv_freq oracle now asserts literal expected values instead of a second call to the same formula the implementation uses; a new test exercises the fix's other half (forward() actually rebuilding cos/sin from the corrected buffer, not just resetting max_seq_len_cached); a new negative test uses transformers' own native 5.x rotary-embedding shape (no dim/base) rather than an unrelated Sequential(Linear, ReLU). make check green throughout (169 passed, 2 skipped, 2 deselected; lint and mypy --strict clean).
Scoped re-review of the prior fix round found the new guard itself (added to catch a future silent regression of fix_rope_inv_freq's duck-typed match) had no test at any tier -- exactly the kind of gap the guard exists to prevent. Adds a fast CPU test that monkeypatches load_model/fix_rope_inv_freq/patch_moe_infer_quantized to construct the guard's actual trigger condition (MoE layers patched, no rope buffers fixed) and asserts the RuntimeError fires. make check green (170 passed, 2 skipped, 2 deselected; lint and mypy --strict clean).
The flat directory had grown to 81 files (7 phases, 0 through 5b) with no structure beyond a date prefix. Moves every file into docs/findings/phase-N/, filenames unchanged, so every cross-reference update is a mechanical directory-prefix rewrite rather than a rename: CHANGELOG.md, CLAUDE.md, README.md, STATUS.md, every plan doc, every runbook, and each findings doc's own cross-references to sibling findings docs. Also repoints the 6 GPU scripts that defaulted --output-dir (or a module-level OUTPUT_DIR) to the bare docs/findings/ at their own specific phase folder (run_baseline.py -> phase-0, run_kernel_bench.py -> phase-1, run_phase3_correctness_gate.py -> phase-3, phase4_concurrency.py/phase4_correctness_gate.py -> phase-4, run_speculative_bench.py -> phase-5b) -- previously all six wrote into one shared, undifferentiated folder. Three gitignored *.safetensors reference-logit files (phase-0, phase-3) moved on disk to match, outside git's tracking. Left untouched: a handful of already-stale filename predictions in planning docs (e.g. a cost-log path with a one-day-off date) that never matched what was actually produced -- pre-existing, not introduced or worsened by this move. make check green throughout (170 passed, 2 skipped, 2 deselected; lint and mypy --strict clean).
…gures README and CLAUDE.md now reflect Phase 5a merged (PR #5), Phase 5b complete (PR #6), the per-phase findings folders, 170 tests, and the running GPU cost ($39.95 across nine sessions). CLAUDE.md's cost discipline gains three rules that each came from a real Phase 5b incident: never leave a pod running across an unbounded wait, pull evidence off the pod before stop, and treat stop as possibly final. Also corrects errors found while re-verifying the Phase 5b findings against the committed results JSONs: the prompt-lookup gate row had mixed in the k-sweep run's numbers (the gate run matched 2 of 4 prompts at 17.7% acceptance and 47.21 tok/s, not 3 of 4 at 14.9% and 44.74); match counts were stated out of 32 instead of 16 (draft-model 13/16, prompt-lookup 9/16); prompt_000 diverges in 6 of 8 sweep configs, not 7; and the summary claimed both drafters beat baseline when draft-model is below it at every k. Adds the direct evidence that the same k=4 prompt-lookup config matched 2/4 prompts in one process and 3/4 in another.
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.
Summary
src/dispatch/speculative/) with two drafters (a real 7B draft model and model-free prompt-lookup), an independent plain-greedy oracle for the baseline, andscripts/run_speculative_bench.py.transformers==5.17.0leaves DeepSeek's remote-code RoPEinv_freqbuffer uninitialized afterfrom_pretrained, poisoning attention with NaN on GPU.fix_rope_inv_freq()now runs insideload_modelfor every caller (previously only one script would have been covered), with a regression guard and results-JSON fields (rope_buffers_fixed,attn_implementation).docs/findings/split intophase-0/...phase-5b/, filenames unchanged; all cross-references and the six GPU scripts' default output dirs updated.Results (A40, bf16, unbatched decode, 64 new tokens, 4 prompts x 5 reps)
token_match(4 prompts)Full k-sweep (k=1,2,4,8, both drafters) in
docs/findings/phase-5b/2026-09-18-phase-5b-speculative-decoding-run.md. Only prompt-lookup beats the baseline (34.4-50.9 tok/s across k=1-8); draft-model is below it at every k (19.6-23.0), despite higher acceptance. Throughputs are single measured runs on one pod; the two k=4 prompt-lookup runs differ by ~5%.Not every config matches the baseline byte-for-byte, and this is reported rather than hidden: In the 8-config k-sweep, draft-model matched the baseline in 13 of 16 (prompt, k) combinations and prompt-lookup in 9 of 16; prompt_000 diverges in 6 of 8 configs, including draft-model at k=1/2/8, and the same k=4 prompt-lookup config matched 2/4 prompts in one process and 3/4 in another. Root-caused live on the GPU to a genuine near-tied logit position (top-2 gap ~0.1-0.4 of ~20) under the int8 kernel's floating-point precision, confirmed by a batch-width probe and a 10-trial same-process determinism probe. The propose/verify/rollback logic was re-derived by hand and is correct; this is a numerical-precision limit, not a logic defect.
Things worth a reviewer's attention
grouped_matmul_int8kernel underlies Phase 5a's "perfect top-1/mutual-top-k agreement" claim, and a single-run top-k check would not detect this near-tie sensitivity. Flagged in the findings doc and STATUS.md for a follow-up decision.results.jsonfiles are exact reconstructions from the session's captured stdout; the 11 rawgenerated-tokens.jsonfiles for the real run were lost when the pod's repo clone did not survive a stop/start (it lived outside the persistent mount). Documented in the findings doc.load_model, guard ordering, test-oracle quality, guard coverage).Test plan
make checkequivalent locally: ruff format/lint clean,mypy --strictclean, 170 passed / 2 skipped / 2 deselected (pytest -m "not gpu")docs/findings/phase-5b/