Skip to content

decode: --lookup-chain 1 and 2 do not change the window cap with the default --suffix-draft - #1305

Closed
ZackO2o wants to merge 1 commit into
Niko1221:mainfrom
ZackO2o:lookup-chain-window-cap-fix
Closed

ZackO2o wants to merge 1 commit into
Niko1221:mainfrom
ZackO2o:lookup-chain-window-cap-fix

Conversation

@ZackO2o

@ZackO2o ZackO2o commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

The change

Size the cap from the cap actually in force:

if (o.lookup_chain > 0 && o.spec >= 2) {
    if (o.mtp_max_t == 0) o.mtp_max_t = o.spec;
    const int chain_base = o.spec;
    o.spec = std::max(chain_base, std::min(std::max(chain_base, o.mtp_max_t) + o.lookup_chain, 8));  // kVerifyMaxT
}

Resulting caps with the default --suffix-draft 3 and --spec 4:

K before after
1 6 7
2 6 8
3 7 8
4+ 8 8

The kVerifyMaxT = 8 ceiling holds, and behavior is unchanged where it should be — verified by evaluating both versions of the arithmetic:

  • --suffix-draft 0: identical for K = 0, 2, 4, 7 (the mtp_max_t == 0 branch still takes over).
  • --spec 6 (already near the ceiling, so the suffix block clamps at 8): identical for K = 1, 2, 3.
  • Maximum reachable cap across K = 0..7 is 8, i.e. the ceiling is not crossed.

This is a one-file, 5-line diff and I have not been able to build or run it. Our box runs a self-built sm_70 engine from a different tree, and building this fork to validate a 5-line change is a larger operation than the change itself; I would rather flag that than claim a test I did not run. The arithmetic above is evaluated directly, and the "before" column is measured on 0.1.40 — that is the limit of what I can honestly assert.

If you would rather not take the code change, the alternative is one line of help text so the behavior is discoverable:

--lookup-chain K     opt-in: after the MTP's drafts, add up to K prompt-lookup drafts that continue
                     them (the window grows to at most 8; default 0 = off)
                     NOTE: with the default --suffix-draft the cap is already --spec+2, so only
                     K > 2 raises it; K beyond the cap's headroom has no effect.

The symptom

With the default --suffix-draft 3, --lookup-chain 1 and --lookup-chain 2 produce a verify window identical to not passing the flag at all. The option is documented as letting a chained lookup "lengthen one [window] by up to K", so a user setting --lookup-chain 2 reasonably expects +2 headroom in the cap. They get +0, silently.

Measured on a 2× V100-PCIE-32GB box (sm_70, CUDA 12 source build, Qwen3.8-Flash-Next IQ3_S), reading the window cap back from the engine's own /metrics engine block (spec is the cap) after a restart per row:

--suffix-draft --lookup-chain spec (window cap) mtp_max
default (unset) 6 4
default 1 6 4
default 2 6 4
default 3 7 4
default 4 8 4
0 4 8 4

--lookup-chain 1 and 2 are indistinguishable from unset. 3 and above do move the cap, but by less than K.

The cause

The two blocks run in sequence in generate.cpp (~line 2089) and the second reads mtp_max_t, which the first has already written:

// suffix drafter, on by default
if (o.suffix_draft > 0 && o.spec >= 2 && o.mtp_max_t == 0) {
    o.mtp_max_t = o.spec;                          // 4
    o.spec = std::min(o.spec + 2, 8);              // 6
}
// --lookup-chain
if (o.lookup_chain > 0 && o.spec >= 2) {
    if (o.mtp_max_t == 0) o.mtp_max_t = o.spec;     // not taken: 4 != 0
    o.spec = std::max(o.spec, std::min(o.mtp_max_t + o.lookup_chain, 8));
    //            = max(6, min(4 + K, 8))          <- K=1,2 give max(6, 5|6) = 6
}

Either the mtp_max_t == 0 guard or the mtp_max_t term is stale by the time block 2 runs. With --spec 4, block 1 leaves spec = 6, so block 2's min(mtp_max_t + K, 8) = min(4 + K, 8) only exceeds 6 once K >= 3 — and even then it is sizing against mtp_max_t (4), not against the spec (6) that was actually set.

The --lookup-chain line of --help says "the window grows to at most 8", which is true of the ceiling but not of the increment; there is no way for a user to discover that small K values are inert in the default configuration.

Why this is worth fixing rather than documenting

The help text implies a monotonic relationship between K and the cap. A user sweeping K looking for the point where chaining starts to pay — which is the natural way to tune an opt-in flag — will see K=1 and K=2 do nothing and conclude, as we initially did, that the feature does not work. Our first write-up of the sweep reported --lookup-chain 2 as a negative result (-1.2% / -3.2% vs default); reading this code afterwards is what showed the window never changed, so that row measured noise in a configuration where the flag could not have helped.

What we measured, and what we did not

…he stale mtp_max_t

With the default --suffix-draft 3 the suffix block above already raises --spec by 2
(4 -> 6) and writes mtp_max_t = 4. The --lookup-chain block then computed
min(mtp_max_t + K, 8) = min(4 + K, 8), so with --spec 4:

  K=1 -> max(6, 5) = 6   (unchanged: the flag does nothing)
  K=2 -> max(6, 6) = 6   (unchanged: the flag does nothing)
  K=3 -> max(6, 7) = 7

Measured on 0.1.40 by reading the cap back from /metrics engine.spec after a restart
per row; K=1 and K=2 were indistinguishable from not passing the flag.

Size from the cap actually in force instead, so K adds up to K of headroom while the
kVerifyMaxT = 8 ceiling and the mtp_max_t bound on the MTP's own drafts are preserved.
Behavior is unchanged when --suffix-draft is 0 or when the cap is already at 8.
@midhatn

midhatn commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

The cap arithmetic checks out, but I think the conclusion that K=1/2 are inert needs narrowing. An unchanged cap does not mean chaining is disabled: with the defaults, the MTP window is 4 and the existing cap is 6, leaving room for 1–2 chained tokens.

I compiled a small Windows C++ probe against the unmodified v0.1.40.1 suffix_drafter.cpp, draft_source.cpp and draft_policy.cpp. Using history 1..35, 100, 1..27, pending MTP drafts 28,29,30, and p_mtp=1, the real lookup and policy functions return:

K=1 current_cap=6 mtp_window=4 proposed=1 kept=1 match=30 first=31 total_window=5
K=2 current_cap=6 mtp_window=4 proposed=2 kept=2 match=30 first=31 total_window=6

This matches the runtime's min(lookup_chain, S-T) path. The patch provides additional capacity, but K=2 can already affect a window without it. I would avoid classifying the previous K=2 throughput result as necessarily noise solely because /metrics.spec was unchanged; proposed/kept/accepted chain counts would establish what happened.

This is a compiled drafter/policy test, not model inference or a speed claim. Increasing the shared cap may still help particular workloads, but that benefit needs measuring separately.

fragtion pushed a commit to fragtion/StrataPool that referenced this pull request Oct 7, 2026
…cap from the cap in force, not the stale mtp_max_t

decode: size --lookup-chain's window cap from the cap in force, not the stale mtp_max_t

With the default --suffix-draft 3 the suffix block above already raises --spec by 2
(4 -> 6) and writes mtp_max_t = 4. The --lookup-chain block then computed
min(mtp_max_t + K, 8) = min(4 + K, 8), so with --spec 4:

  K=1 -> max(6, 5) = 6   (unchanged: the flag does nothing)
  K=2 -> max(6, 6) = 6   (unchanged: the flag does nothing)
  K=3 -> max(6, 7) = 7

Measured on 0.1.40 by reading the cap back from /metrics engine.spec after a restart
per row; K=1 and K=2 were indistinguishable from not passing the flag.

Size from the cap actually in force instead, so K adds up to K of headroom while the
kVerifyMaxT = 8 ceiling and the mtp_max_t bound on the MTP's own drafts are preserved.
Behavior is unchanged when --suffix-draft is 0 or when the cap is already at 8.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0157fjixQD2nRxvCvtXq66DD
@Niko1221

Copy link
Copy Markdown
Owner

Thanks Zack. The fix is right (the flag had no effect next to the default suffix drafter) and is ported with your authorship; it only changes runs that pass --lookup-chain, the default path is untouched (identity gates 10/10 on three boxes).

fragtion added a commit to fragtion/StrataPool that referenced this pull request Oct 17, 2026
Upstream 0.1.45 took in ten of the PRs we carried: Niko1221#567 (prompt encoder), Niko1221#1164 (sibling
borrow), Niko1221#1166, Niko1221#1368, Niko1221#1367, Niko1221#1120 (split overlap), Niko1221#1912, Niko1221#1888, Niko1221#1949, Niko1221#1305.  Their
versions are kept and our copies dropped: verify.cpp's duplicate prelaunch/handoff block,
verify_kernels.hpp, the qsa prompt kernels, gr_upmix_parity, the tokenizer and its tests
(tools/test_strata_tokenizer_upstream.py is gone: upstream's test file is the one now),
the second quantize_scatter declaration.  README credits updated.

Pool parts kept on top:
- conversation cache: upstream renamed the borrow guard (pin -> guard); clear() resets it.
- prefill: the remote_next hand-off beside upstream's hand-off pieces/worker (a pool stage
  has no pieces: the whole chunk goes to the workers).
- generate.cpp: --pool-affinity measure over a node's own layers; the node's dense-range
  load beside STRATA_HC_REQ8; the carve on primary_device over the pool range; checkpoints
  keep pool_id (STRATA_CKPT_POOL/THREADS' parallel save is skipped on a pool PC); the
  workers' retain after eviction and after a resume; the borrow/restore of parked
  conversations on the workers with upstream's --conversation-cache-park-gain; Niko1221#652's
  emit count in upstream's merged windows.
- expert_source: our budget retry beside upstream's lend budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0157fjixQD2nRxvCvtXq66DD
@Niko1221

Copy link
Copy Markdown
Owner

Thanks for the pull request.

Your change is already in main for 0.2.0-beta, with your credit, so there is nothing left for you to do here. We're closing it because we're resetting the tracker for 0.2.0-beta: there are more than 1,300 open issues and pull requests, and we're closing everything that isn't being worked on right now.

If part of your work did not make it in and you want it considered, open a new pull request on top of main that follows CONTRIBUTING.md, or comment /reopen after fixing the title area prefix and the template.

@Niko1221 Niko1221 closed this Oct 19, 2026
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.

3 participants