MTP graph profiles carry no topology class, so one executable serves two attention routes and any draft window past five fails at startup - #221
Conversation
instantiate_graph_family builds one cudaGraphExec_t per topology class and installs every other profile of that class through cudaGraphExecUpdate, which cannot cross a change of node count. dflash_graph_profiles honours that twice: it breaks the frontier where the attention route flips, and derives topology_class from the same predicate. mtp_graph_profiles did neither, so at a draft window of six or more one executable served both the prompt and the chunked small-T route. Today the MTP draft cap of five hides it. With the cap raised - four constants, no other change - draft windows 6 through 15 fail at startup with cudaErrorGraphExecUpdateFailure; with this change all of them run. The shared predicate is renamed verify_uses_chunked_small_t because both planners now read it, and gains the width bound the real route has: causal_attention_resolve_route returns Prompt for every envelope past kMaximumVerifyTokens. verify_route_flip_target locates the boundary by bisecting that predicate rather than restating the route table, which is how the planner and the route could drift apart. At the shipped cap nothing moves: every width up to six resolves to SmallT, so no class changes and the added boundary sits behind a zero check. Adds tests/targets/qwen3_6_35b_a3b/test_mtp_graph_profiles.cpp, which asserts the contract over four capacities, draft windows one to sixteen and all five KV storages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4bad56c5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const std::uint32_t flip_target = verify_route_flip_target(draft_window); | ||
| if (flip_target != 0U) { add_shifted(flip_target, draft_window + 1); } |
There was a problem hiding this comment.
Remove planning for unsupported MTP windows
validate_spec rejects every MTP draft window above kMtpDecodeMaximumDrafts (currently 5), while for every accepted window draft_window + 1 <= 6, so verify_route_flip_target always returns zero and the new topology classification always remains class 0. Consequently, this branch only implements behavior for configurations that cannot reach graph planning; the accompanying test obtains its failures solely by calling the private planner with those invalid windows. Remove this speculative path, or introduce it as part of an explicit product change that raises the MTP limit and updates the complete affected contract.
AGENTS.md reference: AGENTS.md:L59-L61
Useful? React with 👍 / 👎.
|
On the Codex P1: it is right that nothing reaches this today. What I would put on the other side, for you to weigh:
If you would rather the MTP limit stay where it is and the planner not carry a guard for a window it |
Port upstream Neroued#197 (ignore_eos) and Neroued#221 (MTP graph topology class)
instantiate_graph_familybuilds onecudaGraphExec_tper topology class and installs every otherprofile of that class through
cudaGraphExecUpdate, which cannot cross a change of node count.dflash_graph_profileshonours that twice: it breaks the frontier where the attention route flips(
variant.cpp:49-51) and derivestopology_classfrom the same predicate (:114-124).mtp_graph_profiles, three functions above it in the same file, does neither — every profile itreturns carries class 0.
It is a hard failure, not a style point
Raising the MTP draft cap — four constants, no other change — on this build:
error: startup failed | preparing CUDA graphscudaErrorGraphExecUpdateFailure (update result 2)Change
mtp_graph_profilesnow breaks the frontier at the route flip and setstopology_classfrom thesame predicate
dflash_graph_profilesuses.verify_uses_chunked_small_tbecause both planners read it now — asecond copy is what would drift;
causal_attention_resolve_routereturnsPromptforevery envelope past
kMaximumVerifyTokens, so claiming a target dependence beyond width 16 costsa topology class no verify path can request;
verify_route_flip_targetlocates the boundary by bisecting that predicate — monotone inmax_visible_keys— rather than restating the route table, which is how the planner and the routecould drift apart in the first place.
1 file, +46 / −4.
Evidence
New contract test over capacities 2048 / 16384 / 65536 / 262144, draft windows 1..16, all five
KvCacheStoragevalues: no profile may span a route flip inside its own frontier range, and notopology class may carry two routes.
b88c0f6ctestAt the shipped cap it is a no-op
At k ≤ 5 every width resolves to
SmallT, so no class changes and the added boundary sits behindif (flip_target != 0)— the profile list does not move by one element. Measured too: stock caps,greedy, k = 1..5, two binaries of different
md5print byte-identical token ids, 5 of 5, while thesame comparison separates k=1 from k=5.
dflash_graph_profilesis unchanged: the renamed predicate takes the same arguments, and the newwidth bound cannot fire there because DFlash caps at 15.
This does not raise the MTP draft window and proposes no wider window — that needs its own case on
a workload. It removes the crash that meets anyone who raises it, and pins the contract with a test.
RTX 5090
sm_120a, CUDA 13.1, Release,qwen3_6_35b_a3b.ninfer.🤖 Generated with Claude Code