Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions atom/models/qwen4_exp_mtp.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,13 @@ class Qwen4ExpMTP(nn.Module):
disable_fused_shared_loading = True
fused_shared_experts = 0
get_expert_mapping = Qwen4ExpForConditionalGeneration.get_expert_mapping
draft_graph_supports_mrope = True
reuse_draft_graph_step_buffers = True

@staticmethod
def draft_graph_hidden_state_shape(draft_hf) -> tuple[int, ...]:
"""The draft carries the whole hyper-connection bundle between steps."""
return (draft_hf.hc_count, draft_hf.hidden_size)

def __init__(self, atom_config: Config):
super().__init__()
Expand Down
55 changes: 43 additions & 12 deletions atom/spec_decode/draft_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
passes still live with the flavor; only the machine is here.
"""

import math
from collections.abc import Callable, Mapping
from dataclasses import dataclass, field
from typing import TYPE_CHECKING, Any
Expand Down Expand Up @@ -38,10 +39,16 @@ def _owned(value: Any, staged: tuple[torch.Tensor, ...] = ()) -> Any:

@dataclass(frozen=True)
class StagedInput:
"""One staged input's type: everything but the leading batch axis."""
"""One staged input's type: everything but the batch axis.

The batch axis leads unless ``batch_last`` (MRoPE positions, ``[3, N]``):
each batch then views a flat buffer's front as a contiguous ``[*shape, bs]``,
the layout the runner gives its own MRoPE positions.
"""

shape: tuple[int, ...] = ()
dtype: torch.dtype = torch.int32
batch_last: bool = False


@dataclass
Expand Down Expand Up @@ -113,14 +120,21 @@ def bind(self, config: "Config", device) -> "DraftGraph":
)
self._buffers = {
role: torch.zeros(
(config.max_num_seqs, *staged.shape),
(
(config.max_num_seqs * math.prod(staged.shape),)
if staged.batch_last
else (config.max_num_seqs, *staged.shape)
),
dtype=staged.dtype,
device=device,
)
for role, staged in self.inputs.items()
}
return self

def _batch_of(self, role: str, src: torch.Tensor) -> int:
return src.shape[-1] if self.inputs[role].batch_last else src.shape[0]

@property
def will_capture(self) -> bool:
"""Capture only a supported pass when the deployment enables it."""
Expand All @@ -147,7 +161,12 @@ def stage(
How many rows are real comes from the sources, never from the caller: a
count that cannot disagree with the tensor it describes.
"""
counts = {t.shape[0] for t in srcs.values()}
undeclared = sorted(srcs.keys() - self._buffers.keys())
assert not undeclared, (
f"{self.name} has no staged inputs {undeclared}; "
f"declared roles are {sorted(self._buffers)}"
)
counts = {self._batch_of(role, t) for role, t in srcs.items()}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

undeclared role now raises bare KeyError instead of the old assert message? let's keep the diagnostic..

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please fix this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 2daa454. stage() and buffer() now check the roles before anything else, through one _assert_declared. It names each undeclared role and lists the declared ones (... has no staged input 'missing'; declared roles are ['row']) instead of raising a bare KeyError. test_staging_an_undeclared_role_names_it covers it; on 528b871be it fails with KeyError: 'missing'.

assert len(counts) <= 1, (
f"{self.name} was handed batches of {sorted(counts)}; they "
f"describe one step, so one of them is not this step's"
Expand All @@ -167,6 +186,10 @@ def buffer(self, role: str, bs: int) -> torch.Tensor:
f"{self.name} has no staged input {role!r}; "
f"declared roles are {sorted(self._buffers)}"
)
assert not self.inputs[role].batch_last, (
f"{self.name} stages {role!r} batch-last, whose view at one batch is "
f"not the front of a wider one; hand it to stage instead"
)
buf = self._buffers[role]
assert 0 <= bs <= buf.shape[0], (
f"{self.name} requested {bs} rows from draft input {role!r}, "
Expand All @@ -175,7 +198,8 @@ def buffer(self, role: str, bs: int) -> torch.Tensor:
return buf[:bs]

def _stage_one(self, role: str, running_bs: int, src: torch.Tensor | None):
"""Copy ``src`` into this input's fixed buffer, tail-repeating its last row.
"""Copy ``src`` into this input's fixed buffer, tail-repeating its last row
(its last column if ``batch_last``).

Every input repeats the same last index, so a pad row is a coherent copy
of the last real one and only redoes work that row already does. Zeros
Expand All @@ -186,21 +210,25 @@ def _stage_one(self, role: str, running_bs: int, src: torch.Tensor | None):
batch, not a particular one.
"""
buf = self._buffers[role]
out = buf[:running_bs]
shape, batch_last = self.inputs[role].shape, self.inputs[role].batch_last
out = (
buf[: math.prod(shape) * running_bs].view(*shape, running_bs)
if batch_last
else buf[:running_bs]
)
if src is None:
return out
assert src.dtype == buf.dtype, (
f"draft input '{role}' of {self.name} arrived as {src.dtype}, but "
f"its storage is {buf.dtype}; a capture has that address baked"
)
assert src.ndim == buf.ndim, (
assert src.ndim == out.ndim, (
f"draft input '{role}' of {self.name} arrived as {tuple(src.shape)}, "
f"whose leading axis is not the batch; this pass stages "
f"{tuple(buf.shape[1:])} per sequence. MRoPE positions ([3, N]) are "
f"the case that reaches here"
f"whose {'last' if batch_last else 'leading'} axis is not the batch; "
f"this pass stages {shape} per sequence"
)
scheduled_bs = src.shape[0]
dst = out[:scheduled_bs]
scheduled_bs = self._batch_of(role, src)
dst = out[..., :scheduled_bs] if batch_last else out[:scheduled_bs]
assert src.shape == dst.shape, (
f"draft input '{role}' of {self.name} arrived as {tuple(src.shape)}, "
f"but its fixed storage expects {tuple(dst.shape)}"
Expand All @@ -210,7 +238,10 @@ def _stage_one(self, role: str, running_bs: int, src: torch.Tensor | None):
if src.data_ptr() != dst.data_ptr() or src.stride() != dst.stride():

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

batch_last + producer fill gives an overlapping self-copy with wrong strides.. assert it or document it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@valar-cop I followed your suggestion on atom/spec_decode/draft_graph.py:234. It's both asserted and documented now. cc @valarLip

You're right that it's real. On CPU I filled buffer("pos", 2) and staged at running_bs=4. That took the copy branch (same data_ptr, strides (2, 1) vs (4, 1)) and scrambled rows 1 and 2 of the positions without any error. 3→8 and 5→6 do the same.

  • _stage_one now asserts when the source is a view inside the role's own fixed storage but isn't the exact view it writes. The exact view (same pointer and stride) still skips the self-copy.
  • buffer's docstring states the invariant. A leading-batch producer fills buffer(role, scheduled_bs). A batch_last producer fills buffer(role, running_bs)[..., :scheduled_bs], which has the staged stride.
  • I asserted rather than cloned because no caller reaches this. The only batch_last input is the MRoPE positions, which _enter_decode_metadata builds with a fresh index_select, and nothing fills them through buffer.

(commit 528b871be; new tests test_a_batch_last_producer_fills_the_running_view and test_a_batch_last_fill_of_a_narrower_view_is_refused in tests/test_draft_graph.py, and the second one fails on the old guard. CPU: the draft-related suites pass, 180 passed / 46 skipped; black and ruff are clean.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm. the guard now refuses before dst.copy_, and the 0 <= ptr - buf range check catches the narrow-view overlap that used to scramble rows.

checked the callers: only hidden_states (eagle_proposer.py:778) and the dspark/v4 mono anchors fill a buffer, all leading-batch at running_bs, and positions comes from a fresh index_select (eagle_proposer.py:551), so nothing trips it today. assert over clone is fine.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

stride differs when running_bs != scheduled_bs, so this guard takes the copy branch over overlapping storage.. latent today but hidden_states would corrupt silently. assert no alias or copy via clone?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@valar-cop I followed your suggestion on atom/spec_decode/draft_graph.py:234: it now asserts there is no alias, in the same commit as the other thread. cc @valarLip

One correction on scope: hidden_states can't hit this. It's a leading-batch input, so buffer("hidden_states", bs) is buf[:bs], and its strides don't depend on bs. At running_bs != scheduled_bs the producer's view is exactly out[:scheduled_bs], so the self-copy is skipped. I checked this on CPU at 2→4, 3→8 and 5→6: same pointer, same strides, and both the real rows and the pad tail come out correct. Only a batch_last input has a batch-dependent stride.

Every source that lives inside a staged buffer today is the exact view:

  • EagleProposer's step-0 hidden_states, which index_select writes into buffer(..., scheduled_bs).
  • The next step's graphed_ids[:scheduled_bs] and ret_hidden_states[:scheduled_bs].
  • DSpark's anchors, which are filled at buffer(..., running_bs) and staged at running_bs.

So the assert doesn't fire on any current path, and every copy that ran before still runs. A future alias fails loudly instead of corrupting.

(commit 528b871be; test_a_draft_step_wider_than_the_schedule_pads_every_input stages ids, MRoPE positions and producer-filled hidden states at scheduled_bs=3 / running_bs=8, both eager and replayed. It checks the real rows, the pad tail and the fixed addresses.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified against the code: hidden_states is a leading-batch input (StagedInput(shape=hidden_shape), eagle_proposer.py:159), so buffer(role, scheduled_bs) is always buf[:scheduled_bs], and dst is that same slice at any running_bs — only the batch-last view has a stride that moves with bs. The assert plus the narrow-view test is the right shape.

lgtm, my mistake on hidden_states.

dst.copy_(src)
if running_bs > scheduled_bs:
out[scheduled_bs:].copy_(src[-1]) # broadcasts; no expand needed
if batch_last:
out[..., scheduled_bs:].copy_(src[..., -1:])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

docstring still says row, now it's the last real sequence for batch_last.. please reword

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@valar-cop I followed your suggestion on atom/spec_decode/draft_graph.py:237. cc @valarLip

The _stage_one docstring now says it tail-repeats the last real sequence, which is the last row, or the last column of a batch_last input (the [3, N] MRoPE positions). The rest of the paragraph says "sequence" instead of "row".

(commit 4b28fdd56, docstring only; the head is now 80bd51de8 after a merge of main. black and ruff are clean. On CPU, tests/test_draft_graph.py passes 28/28 and tests/test_glm5_next_mtp_routing.py passes 2, with 5 skipped for aiter. test_qwen4_exp_mtp, test_dspark and test_region_graph skip on CPU because they need triton, aiter or a GPU. Pre Checkin is green on 80bd51de8.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

yes, that reads right now, pad repeats the last real sequence in both layouts and tests/test_draft_graph.py:195 pins the batch-last one.

lgtm from me. note the vllm accuracy matrix is still skipped on this head, it needs the approval or a ci:full label to run.

else:
out[scheduled_bs:].copy_(src[-1]) # broadcasts; no expand needed
return out

def is_captured(self, running_bs: int) -> bool:
Expand Down
17 changes: 11 additions & 6 deletions atom/spec_decode/eagle_proposer.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,14 @@ def _declare_draft_graphs(self):
pad and nothing a graph could pin. Declaring anyway used to warm and
capture a ONE-dimensional-positions shape that serving never runs --
graphs nobody replays, paid for at every startup.
A draft model that sets ``draft_graph_supports_mrope`` is the exception:
it stages them batch-last.
"""
self.step = None
if self.runner.use_mrope or self.mtp_k < 2:
mrope = self.runner.use_mrope
if (
mrope and not getattr(self.model, "draft_graph_supports_mrope", False)
) or self.mtp_k < 2:
# mtp_k == 1 has no step 1+, so warming one would capture a graph
# `propose` can never reach.
self._reuse_step_buffers = False
Expand All @@ -148,7 +153,11 @@ def _declare_draft_graphs(self):
# `input_ids` from that buffer to this one, and `stage` asserts the
# two agree.
"input_ids": StagedInput(dtype=torch.int32),
"positions": StagedInput(dtype=torch.int64),
"positions": (
StagedInput(shape=(3,), dtype=torch.int64, batch_last=True)
if mrope
else StagedInput(dtype=torch.int64)
),
"hidden_states": StagedInput(
shape=hidden_shape,
dtype=self.dtype,
Expand Down Expand Up @@ -809,10 +818,6 @@ def propose(
# from it, so it has to be `running_bs` long before THAT --
# padding it with the other rows, just before the
# forward, is a step too late.
#
# No MRoPE case to guard: a model whose positions keep
# the token axis last declares no pass, so `self.step` is
# None and these stay the target's own 2-D positions.
if self.step is not None:
positions = self.step.stage(
running_bs, {"positions": positions}
Expand Down
53 changes: 51 additions & 2 deletions tests/test_draft_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,14 @@ def test_fixed_storage_accessor_checks_role_and_capacity():
g.buffer("row", 257)


def test_staging_an_undeclared_role_names_it():
g = _graph()
with pytest.raises(
AssertionError, match=r"\['missing'\]; declared roles are \['row'\]"
):
g.stage(1, {"missing": torch.zeros(1, dtype=torch.int32)})


def test_staging_rejects_a_wrong_trailing_shape_before_identity_check():
g = _graph(inputs={"row": StagedInput(shape=(4,), dtype=torch.int32)})
wrong = g._buffers["row"][:2, :1]
Expand All @@ -171,10 +179,10 @@ def test_staging_a_wrong_dtype_fails_loudly():


def test_staging_a_source_whose_leading_axis_is_not_the_batch_fails_loudly():
"""Every staged source is indexed as `[rows, *StagedInput.shape]`.
"""Every leading-batch source is indexed as `[rows, *StagedInput.shape]`.

MRoPE positions are the live case: they keep the token axis LAST (`[3, N]`),
so a drafter must not hand them to a pass at all. Without this the mismatch
so a drafter must declare them `batch_last`. Without this the mismatch
surfaced as a `copy_` size error two frames down, which reads like a
batch-size bug rather than a layout one.
"""
Expand All @@ -183,6 +191,36 @@ def test_staging_a_source_whose_leading_axis_is_not_the_batch_fails_loudly():
g._stage_one("row", 8, torch.zeros(3, 8, dtype=torch.int32))


def _mrope_graph():
return _graph(
inputs={
"pos": StagedInput(shape=(3,), dtype=torch.int64, batch_last=True),
"row": StagedInput(dtype=torch.int32),
}
)


def test_batch_last_input_is_contiguous_and_tail_repeats_the_last_column():
"""MRoPE positions stage as a contiguous `[3, bs]`, the runner's own layout,
beside a leading-batch input a producer filled in place (`hidden_states`)."""
g = _mrope_graph()
src = torch.arange(6, dtype=torch.int64).view(3, 2) * 10
g.buffer("row", 2).copy_(torch.tensor([1, 2], dtype=torch.int32))
staged = g.stage(4, {"pos": src, "row": g.buffer("row", 2)})
pos = staged["pos"]
assert pos.shape == (3, 4) and pos.is_contiguous()
assert pos.tolist() == [[0, 10, 10, 10], [20, 30, 30, 30], [40, 50, 50, 50]]
assert staged["row"].tolist() == [1, 2, 2, 2]
assert pos.data_ptr() == g._buffers["pos"].data_ptr()


def test_a_batch_last_input_has_no_producer_view():
"""Its view at `scheduled_bs` overlaps the `running_bs` one at another
stride, so staging a fill of it would copy the storage onto itself."""
with pytest.raises(AssertionError, match="batch-last"):
_mrope_graph().buffer("pos", 2)


def test_sources_that_disagree_on_the_batch_are_refused():
"""They describe one step, so one of them is not this step's."""
g = _graph(
Expand All @@ -201,6 +239,17 @@ def test_sources_that_disagree_on_the_batch_are_refused():
)


def test_batch_last_sources_must_agree_with_leading_ones():
with pytest.raises(AssertionError, match="batches of"):
_mrope_graph().stage(
4,
{
"pos": torch.zeros(3, 2, dtype=torch.int64),
"row": torch.zeros(3, dtype=torch.int32),
},
)


def test_warmup_runs_the_epilogue_too_not_just_the_capturable_forward(monkeypatch):
"""Warming only the capturable forward leaves the rest of the pass cold.

Expand Down
32 changes: 32 additions & 0 deletions tests/test_qwen4_exp_mtp.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
qsa_select_paged_tokens,
qsa_sparse_paged_gqa,
)
from atom.spec_decode.draft_graph import StagedInput


def test_qwen_mtp_does_not_import_deepseek_when_pcp_disabled(monkeypatch):
Expand Down Expand Up @@ -290,6 +291,37 @@ def prepare(self, batch, running_bs, running_tokens, max_seqlen_q):
assert not graph_positions[:, tokens:].count_nonzero()


@pytest.mark.parametrize("opted_in", [True, False])
def test_mrope_draft_graph_declares_positions_batch_last(opted_in):
"""Qwen3.8's MTP head declares its steps 1+ with `[3, bs]` positions; an
MRoPE draft model that has not opted in still declares no pass."""
from atom.models.qwen4_exp_mtp import Qwen4ExpMTP
from atom.spec_decode.eagle_proposer import EagleProposer

proposer = SimpleNamespace(
runner=SimpleNamespace(use_mrope=True),
mtp_k=3,
speculative_config=SimpleNamespace(
draft_model_hf_config=SimpleNamespace(hidden_size=8, hc_count=2)
),
model=object.__new__(Qwen4ExpMTP) if opted_in else SimpleNamespace(),
dtype=torch.bfloat16,
_step_forward=lambda *a, **kw: None,
_step_head=lambda *a, **kw: None,
_step_warmup_inputs=lambda *a, **kw: None,
)
graphs = EagleProposer._declare_draft_graphs(proposer)
if not opted_in:
assert graphs == () and proposer.step is None
return
(graph,) = graphs
assert graph.inputs["positions"] == StagedInput(
shape=(3,), dtype=torch.int64, batch_last=True
)
assert graph.inputs["hidden_states"].shape == (2, 8)
assert proposer._reuse_step_buffers


def test_draft_qsa_view_drops_only_graph_padding():
from atom.model_ops.attentions.qwen4_exp_attn import Qwen4ExpQSAMetadata

Expand Down
Loading