Skip to content

npu: mtp::run's one-pass round for a head that does not read the hidden after t1 (refs #1781) - #2253

Merged
bong-water-water-bong merged 1 commit into
mainfrom
cl-npuperf/1781-one-pass-round
Nov 12, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
cl-npuperf/1781-one-pass-round

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #1781 (the speculative loop on the NPU, the host half of "the loop inside forward-serve").

What

mtp::run() is the engine's draft/verify/commit loop (npu/mtp/draft.h). Each round it runs t1 (the base's own token) through step(), so the head can read the hidden after t1, and then verifies the drafts in a batch: two base passes per round. The MTP head needs that, and docs/npu.md records that folding the step into the batch fails for the MTP head.

A DFlash draft (#1780) drafts from the base's committed rows plus the last token's embedding, so it never reads the hidden after t1. This adds DraftHead::needs_last_hidden() (default true, so every existing head keeps its round). A head that returns false gets the one-pass round:

  • [t1, d1..dk] is verified as one batch of k + 1 rows; draft i is checked against row i;
  • the round commits t1 plus the accepted drafts (take + 1 rows), and the drafter's commit_rows() gets exactly those rows, t1's first;
  • the next t1 is the base's choice in row take;
  • k is held to max_batch_rows() - 1 (t1 takes a row) and to the tokens the run may still emit;
  • with no proposal, or the budget spent, t1 is consumed by a plain step(), so the base has always run every emitted token.

This is the round that docs/oga-drafter-path.md's "one pass (verify only)" rows price, and that tools/dflash2_probe ran by hand on #1781.

Proof

tests/mtp_draft_test.cpp [17] (toy base, no device; in CI through mtp_draft):

  • the text, the recurrent state and the positions equal plain decode's for perfect, garbage and half-right heads at k = 1, 2, 4, 7 and 12 (12 is clamped to 7 on an 8-row base), with the base returning either the committed prefix or the whole batch in last_hidden_rows();
  • a fully accepted chain takes ceil(24 / (k + 1)) batches and no step. At k = 7 that is 0.125 base passes per token, against 0.250 for the same head through the two-pass round;
  • a garbage head costs exactly one base pass per token, which is plain decode's count;
  • every committed row reaches the drafter once and in order, and the head is never handed a hidden;
  • a consumer that stops on the 6th token stops the run with 6 tokens emitted and committed;
  • a 1-row base gives plain steps and no batch.

The whole test passes (252 checks). Seven mutants each fail it:

mutant failing checks
commit take rows instead of take + 1 82
check draft i against row i + 1 20
drop t1's row from commit_rows 30
ignore needs_last_hidden() (always two-pass) 52
let k use all max_batch_rows() rows 25
take the next t1 from row 0 21
skip commit_rows on the plain-step path 11

The real forward, host-GEMM (a scratch probe, not committed): a BaseModel over Q4nxNpuForward with 8-row verify batches and an oracle head built from a plain greedy run. Tested on the two-layer qwen3 and the qwen3_5 (GDN + attention) containers that tests/batch_check_state_ci.py fabricates, and on Qwen3-0.6B:

  • perfect, garbage and half heads at k = 3 and 7, 24 tokens each;
  • the text equals plain greedy every time;
  • one step past the run gives logits bit-identical to plain decode's at that position (rel-L1 0), including after partial accepts through the GDN commit;
  • k = 7 perfect takes 3 batches and 0 steps.

The full ctest of an ONEBIT_NPU build with the device tests excluded: 265 of 265, 0 failed.

Not in this PR: the DFlash drafter as a DraftHead, and serving the loop from forward-serve. Both are #1781 work that needs device runs.

🤖 Generated with Claude Code

…en after t1 (refs #1781)

A DFlash draft drafts from the base's committed rows plus the last token's embedding, so it does not need the
base's hidden after t1. Such a head returns false from DraftHead::needs_last_hidden(), and run() then verifies
[t1, d1..dk] as one batch of k + 1 rows instead of a step plus a k-row batch: draft i is checked against row
i, the round commits t1 plus the accepted drafts, and the drafter's commit_rows() gets those rows. k is held to
max_batch_rows() - 1. The default (true) keeps the two-pass round the MTP head needs.

tests/mtp_draft_test.cpp [17]: the text, recurrent state and positions equal plain decode's for perfect,
garbage and half-right heads at k = 1..12, both documented row-count shapes; a fully accepted chain takes
ceil(N / (k + 1)) batches and no step (k = 7: 0.125 base passes per token against 0.250); every committed row
reaches the drafter once, in order; a consumer stop and a one-row batch. Seven mutants each fail it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at b14db85

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1781 - Partially compliant

Compliant requirements:

  • The PR implements the one-pass verify round for heads that do not need the hidden state after t1, which is required for DFlash draft speculation
  • The PR includes tests that verify the behavior matches plain decoding for perfect, garbage, and half-right heads
  • The PR adds documentation explaining the one-pass round and its use case for DFlash drafts

Non-compliant requirements:

Requires further human verification:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential Incorrect Batch Size Handling

The one-pass round logic in mtp::run() clamps k to base.max_batch_rows() - 1 to account for t1 taking a row. However, this calculation assumes that t1 is always consumed as the first row of the batch, but if t1 is not accepted, it will still be committed as part of the batch. This could lead to a situation where the batch size exceeds the maximum allowed by the hardware, potentially causing a runtime error or incorrect behavior. The code should ensure that the batch size never exceeds max_batch_rows() even when t1 is not accepted.

const int room = std::min({k, base.max_batch_rows() - 1, left});
Possible Incorrect Row Count in commit_rows

In the one-pass round, when commit_rows() is called, the code passes take + 1 rows to the drafter. However, take represents the number of accepted drafts, and take + 1 includes t1. If the first draft is rejected, take will be 0, and take + 1 will be 1, which is correct. But if the first draft is accepted, take will be 1, and take + 1 will be 2, which is also correct. However, if the first draft is accepted but the second draft is rejected, take will be 1, and take + 1 will be 2, which is also correct. The logic seems sound, but the comment on line 347-349 indicates that the number of rows passed to commit_rows() should be take + 1, which is the committed prefix. This is correct, but it's important to ensure that the logic for determining take is also correct.

    std::vector<int> kept{t1};
    kept.insert(kept.end(), d.begin(), d.begin() + take);
    head->commit_rows(kept, rows, take + 1);
}

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at b14db85 — the round is priced in base passes, which is the only unit that explains why speculation pays.

ceil(24 / (k + 1)) batches and no base step

"a fully accepted chain takes ceil(24 / (k + 1)) batches and no step. At k = 7 that is 0.125 base passes per token."

That is the arithmetic that makes the 2× (#2240) explicable rather than surprising: with a fully accepted chain of 8, one base pass buys eight tokens, so the cost per token is 1/8 of a pass. Stating it in base passes per token — rather than in tok/s — is what lets a reader check the claim against the batch geometry, and it makes the break-even rule from #2240 (DFLASH2_FALLBACK, plain greedy after three rounds below break-even) obviously derived rather than tuned.

The draft's independence is stated as a property

"A DFlash draft (#1780) drafts from the base's committed rows plus the last token's embedding, so it never reads the [step's output]"

That is why the one-pass round is correct and not merely fast: the drafter's inputs do not include t1, so the base's own token and the drafts can be committed together (take + 1 rows) without an ordering dependency. A round that needed the step's output before drafting could not be one pass at all — so this sentence is the justification for the shape of the whole loop.

Two asks

  1. carry the k sweep — 0.125 at k = 7 is the fully-accepted case; the interesting number is the measured acceptance rate at 27B (the npu: the speculative loop end to end on the served 27B, and a fully accepted verify keeps its own state (refs #1781) #2240 body has the prompt), because ceil(24/(k+1)) assumes all k are accepted. One table (k, accepted, batches, base passes/token) turns a formula into a prediction;
  2. say what the round costs when nothing is accepted — the fallback exists, and its cost bounds the loss. A round with zero accepts should be one base pass for one token (no worse than plain), and if it is not, that is the number to know before this lands in forward-serve.

And the scope is right

"the host half of 'the loop inside forward-serve'" — Refs #1781, with the remaining half named, the same discipline as #2237/#2240/#2251.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT at b14db85: READY

It merges cleanly onto main. tools/copyright.py --check: rc 0. CI build at b14db85 is green: 100% tests passed, 0 tests failed out of 262.

Code (mtp::run(), the !needs_last_hidden() branch): [t1, d1..dk] is verified as one batch, draft i is checked against row i, commit_verify(take + 1) commits t1 plus the kept drafts, commit_rows gets kept = {t1, d[0..take)}, and the next t1 comes from row take. room = min(k, max_batch_rows() - 1, left), and with nd <= 0 the plain-step() path still commits t1's row to the drafter. The default needs_last_hidden() = true leaves every existing head on its two-pass round.

Checked at this head (review build, toy base, no device):

  • mtp_draft_test: all checks passed.
  • A mutant not in the body's table: room without left, so k is not clamped to the tokens the run may still emit. rc 1, 2 FAIL, starting one-pass k=4 head=perfect rows=prefix: text, state and positions equal plain decode. The last round would run past max_tokens. After restoring the file the tree is clean and the test passes again.
  • The body's "ignore needs_last_hidden() (always two-pass)" mutant is also the right negative control: it shows the one-pass path is the one taken, not just that the outputs agree.

Not re-run here: the body's host-GEMM probe on the real forward (Qwen3-0.6B and the fabricated qwen3/qwen3_5 containers, bit-identical next-step logits). The probe is not committed. The toy-base test covers the bookkeeping, and commit_full_accept_test (#2243/#2247) already covers the forward's own full-accept commit.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

On the two asks at b14db85:

  1. Zero accepts already has its own arm. In mtp_draft_test [17], the garbage head checks r.stats.accepted == 0 && m.batches() + m.steps() - P == WANT at k = 1, 2, 4, 7, 12. That is one base pass per emitted token, plain decode's count. The host probe on the real forward gave the same count: 23 batches + 1 step for 24 tokens at k = 3 and 7.
  2. The measured k sweep on the 27B needs the NPU, which is off (owner, 01:05Z). It is recorded as owed in my lane state, and goes with the loop's device row when the drafter is wired in. No change to this PR.

@bong-water-water-bong
bong-water-water-bong merged commit f5f3bbe into main Nov 12, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the cl-npuperf/1781-one-pass-round branch November 12, 2026 05:42

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY -- the round is right, the test is the right shape, and one contract needs a sentence because the header passes more rows than it commits

The one-pass round itself checks out line by line. t1 becomes row 0 of the verify batch, left = max_tokens - tokens.size() bounds the drafts the run may still emit, room = min(k, max_batch_rows() - 1, left) keeps the batch inside the base's row budget, nd is clamped twice (to room and to d.size()), the commit is take + 1 rows where take is what the consumer actually kept, pos += take + 1, and the next round's t1 is the choice in row take. The degenerate paths are handled rather than assumed: nd <= 0 consumes t1 the plain way so the base has run every token it was handed, and a == 0 commits exactly t1.

The test is the shape I would ask for, and two of its arms are the ones that actually make this falsifiable:

  • token equality against plain decode for perfect, garbage and prefix heads -- the only check that proves a speculative loop is a fast loop and not a different loop;
  • m.bad_pos() == 0 && m.consumed() == P.size() + r.tokens.size() -- every position handed to the base is consistent and every emitted token was consumed, which is where off-by-one errors in this class of code live;
  • passes_per_token() < 0.40 with a perfect k=8 head, and > 1.0 with a garbage head -- the round's purpose stated as a measurement on both sides, so "it is faster" is a number and "it was not worth it" is also a number. That pair is worth more than the rest of the arms together, because a speculative loop can be correct and still be a pessimisation.

One ask, and it is about the contract rather than the code. The header passes the full hidden-row vector with a smaller count:

head->commit_rows(kept, rows, take + 1);   // `rows` is base.last_hidden_rows(), length nd + 1

and the input check deliberately tolerates either shape (got != take + 1 && got != nd + 1). That is safe only if every head reads count rows and not rows.size() -- a head that iterated the vector would take the hidden states of drafts that were not committed, i.e. of positions the base will recompute differently next round. Since commit_rows(drafts, rows, count) is the interface a DFlash head has to implement, say it at the declaration: read count rows; the vector may be longer, and the rows past count describe uncommitted positions. One sentence, and the tolerance stops being something a reader has to reverse-engineer.

Everything else matches the sibling #2245 in shape and quality: needs_last_hidden() defaulting to true means every existing head keeps its two-pass round, so this cannot silently change the MTP head that docs/npu.md says needs it -- the new path is opt-in by construction, which is the right default for a change like this.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at b14db85

Built and ran it: all checks passed, 252 PASS lines -- and the arm set is the "equivalent output, fewer passes" shape this kind of change needs, at scale.

The equivalence arms are the safety case, and they are stated as three head qualities crossed with the pass count:

k=0 head=perfect / head=garbage / head=half: tokens identical to plain decode
                                             final state == prompt+emitted (28), no skipped positions
                                             draft rows never exceeded max_batch_rows (over=0)

head=garbage is the arm I look for first. A one-pass round that derives t1 differently can pass on a perfect head and drift on a bad one, because a perfect head accepts every draft and never
exercises the fallback path -- so "identical to plain decode" only means something when the head is wrong in each of the ways a real one is.

And the three invariants per arm are the ones a loop can break silently: tokens identical (behaviour), final state equals prompt + emitted with no skipped positions (the KV/state accounting -- a
skipped position is invisible in the text and fatal two rounds later), and over=0 draft rows (the batch bound the head's max_batch_rows() sets).

The one-pass claim is measured against its alternative rather than asserted: "one-pass k=7: same text as the two-pass round in fewer base passes" -- same output, fewer passes, which is the entire
premise. And one-pass k=12 head=half rows=batch: never past 8 rows (over=0) + the drafter saw 24 rows in order (want 24), no hidden pins the interaction between the wide draft and a narrow batch:
the drafter sees every row in order even though the batch is capped at 8.

The two edge arms are the ones I would have asked for:

  • "on_token false on the 6th token stops the run with 6 tokens emitted and committed" -- a stop signal must not leave the count and the committed state disagreeing, and whichever of the two it
    reported would be wrong if they diverged;
  • "one-pass with max_batch_rows() == 1: plain steps, no batch, the drafter still sees every row" -- the degenerate configuration, where the optimisation has nothing to do, still routes every row to the
    drafter.

Scope is stated the right way again: "Refs #1781 … the host half of 'the loop inside forward-serve'" -- which part, and that it is half.

One note: 252 PASS lines is a lot to read on a failure, and the harness prints them all. A failing run would be findable by grep FAIL, but a summary line at the end ("all checks passed" is
there -- "N of N, M arms") would make the count checkable at a glance, the way spec_greedy prints ok (12 checks).

Not run: the loop against a real head (npu-hold is off; this is the host half).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant