Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 27 additions & 0 deletions atom/compass/runner/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# SPDX-License-Identifier: MIT
# Copyright (C) 2024-2025, Advanced Micro Devices, Inc. All rights reserved.

"""The model runner a simulated run installs in place of ATOM's own.

Two modules, split by what each is allowed to import:

- `overrides` holds the behaviour -- the methods that keep construction off the
device -- and imports nothing from the engine. That is not a stylistic
preference. Importing `atom.model_engine.model_runner` runs aiter's
architecture probe, which shells out to `rocminfo` and raises in a container
with no driver, so anything reachable from that import cannot be exercised by
a test that runs without one.
- `model_runner` binds those overrides onto `ModelRunner`, and is therefore the
only module here that needs a driver to import.

This package stays empty at import time for the same reason, so naming it costs
nothing.

`COMPASS_RUNNER_QUALNAME` is the string that selects the runner: a `Config`
field, read where the worker process instantiates its runner. Installing this
class needs no change to ATOM.
"""

COMPASS_RUNNER_QUALNAME = "atom.compass.runner.model_runner.CompassModelRunner"

__all__ = ["COMPASS_RUNNER_QUALNAME"]
31 changes: 31 additions & 0 deletions atom/compass/runner/model_runner.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# SPDX-License-Identifier: MIT
# Copyright (C) 2024-2025, Advanced Micro Devices, Inc. All rights reserved.

"""The runner class that `runner_qualname` names."""

from __future__ import annotations

from atom.compass.runner.overrides import NonAllocatingRunner
from atom.model_engine.model_runner import ModelRunner


class CompassModelRunner(NonAllocatingRunner, ModelRunner):
"""A `ModelRunner` that constructs without weights, KV tensors or a step.

Everything the engine does around a step -- the scheduler, admission, the
block manager, the prefix index -- is ATOM's own and runs unchanged. What
this class removes is the weights, the KV tensors and the step itself, by
overriding the five methods that own them; see `overrides`, which holds the
bodies and says why each one does what it does.

Construction is not free of device memory. What remains is the base's
forward-vars ring from `allocate_forward_vars`, whose dominant term is a
`max_num_batched_tokens` by `hidden_size` output buffer. It is sized by the
batch budget and the model's hidden size, not by the model's weights, and
no named tensor on the runner holds any of it.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

F15 — non-blocking, and the one clause of this sentence I read as false. Taking the digits out took two bounds out with them.

This is against the round-3 delta, not a reopening of F12 — F12 is closed and the "hidden size, not weights" clause is correct now. But round 2's paragraph carried its bounds inside its digits, and two of them left when the digits did.

(a) no named tensor on the runner holds any of it. Round 1's instrument reported two disjoint readings side by side:

named cuda tensors on the runner: 0 bytes
forward_vars gpu bytes:  2,147,940 / 21 entries   (99.1% of the residue)

The 0-byte reading was of a set that excluded the ring — the ring is the next line. This sentence takes the reading of set A and binds it to set B with "any of it", where "it" is the ring. On the plain reading it is false, and the references are nameable:

  • self.forward_vars = {...} — atom/model_engine/model_runner.py:1297, from allocate_forward_vars, which __init__ calls at :777
  • self._fv_ring = [self.forward_vars] — :1358 (also :1383), which is literally the "ring" this sentence names

A constructed runner holds 100% of those bytes under two named attributes for its whole lifetime, and one is named after the thing the sentence is about. The clause is true only under "a tensor bound directly to an attribute name" — the instrument's definition, and exactly the condition a docstring cannot carry. That is the round-2 ruling's own failure mode, surviving in the one clause that is still a measurement with its digits filed off.

The claim the reading actually supports is more useful to #77/#78 anyway: this subclass adds no tensor of its own. NonAllocatingRunner sets only self.model (an UnbuiltModel registering no parameter and no buffer, overrides.py:39-56, :69) and self.config.num_kvcache_blocks (overrides.py:111). Suggested:

...not by the model's weights, and this class adds no tensor of its own — the ring is the base's, held on the runner for its lifetime.

(b) The lesser one. What remains is the base's forward-vars ring was measured at 99.1% / 99.4%, and round 2's sentence said so. The PR body still names the other 0.6–0.9% (torch.cuda.Stream, torch.cuda.Event, the attention builder, initialize_eplb_runtime); this line now says it is not there. One word: "Almost all of what remains…".

The other three clauses I checked and accept without qualification, including dominant term — that is a structural property of the expression (outputs is max_num_batched_tokens x hidden_size x itemsize at :1308-1313; every other entry is max_bs-sized or max_num_batched_tokens x 8), checkable from the code with no conditions, so I do not read it as a proportion with the proportion removed.

Not blocking, and the standalone comment says why I considered it and declined: it is a precision defect, not a false number, nothing in the diff behaves differently, and a third blocking round is an escalation under AI_DEV_RULES.md that would halt the base of the runner chain over eight words. If anything else is pushed to this branch, fold it in; otherwise #90 should carry it, since it edits this file.


`NonAllocatingRunner` comes first so its methods win over the base's. There
is deliberately no `__init__`: the base runs all of its own before a
subclass body would get control, so there is no point at which state set
here would be visible to it.
"""
128 changes: 128 additions & 0 deletions atom/compass/runner/overrides.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
# SPDX-License-Identifier: MIT
# Copyright (C) 2024-2025, Advanced Micro Devices, Inc. All rights reserved.

"""The model-runner methods that keep construction off the device.

These are the memory-owning methods of ATOM's `ModelRunner`: the ones that read
a checkpoint, warm the model, size the KV pool, allocate it, and run a step.
They live here rather than beside the subclass because this module imports
nothing from the engine, and so can be executed on a machine with no driver.

Two of them run during construction (`_build_and_load_model`, `_maybe_warmup`)
and decline to do their work. The rest run afterwards, over the worker's RPC
channel; `allocate_kv_cache` does the arithmetic and none of the allocation,
and the other two refuse by name because their answers are not this module's to
invent -- a wrong duration or a wrong block count would be indistinguishable
from a measured one.

Order matters when mixing this in: `NonAllocatingRunner` must precede
`ModelRunner` in the bases so these definitions win. The class deliberately
defines no `__init__`. The base class runs the whole of its own `__init__`
before a subclass body would get control, so anything these methods read has to
come from `self.config`, which the base sets first.
"""

from __future__ import annotations

import logging
from typing import Any

import torch

logger = logging.getLogger(__name__)


class RunnerRefusal(RuntimeError):
"""Raised where this runner has no answer and will not invent one."""


class UnbuiltModel(torch.nn.Module):
"""Stands in for the module tree a non-allocating runner never builds.

It registers no parameter and no buffer, so it costs nothing on any device,
and calling it raises instead of returning something a caller could mistake
for a forward pass.
"""

def __init__(self, model_class: Any) -> None:
super().__init__()
self.model_class_name = getattr(model_class, "__name__", repr(model_class))

def forward(self, *args: Any, **kwargs: Any) -> Any:
raise RunnerRefusal(
f"{self.model_class_name} was never built: this runner predicts what a "
"forward pass costs instead of running one, so there are no weights "
"to call."
)


class NonAllocatingRunner:
"""Overrides that construct a model runner without touching device memory."""

def _build_and_load_model(self, model_class: Any) -> None:
"""Build nothing and read no checkpoint.

The base class constructs the model with the default device set to this
rank's GPU and then fills it from disk. Both halves are skipped here, so
no weight byte reaches the device and no checkpoint is read.
"""
self.model = UnbuiltModel(model_class)
# Cleared on the way out, as both of ATOM's own implementations do: the
# caller set the default device to this rank's GPU before calling, and
# the code that runs next is written against a cleared default.
torch.set_default_device(None)
logger.info(
"%s not built and no checkpoint read; no weight bytes on the device.",
self.model.model_class_name,
)

def _maybe_warmup(self) -> None:
"""Skip warmup.

Warmup runs a forward over a dummy batch against real weights. There are
none here, so there is nothing to warm. Skipping is also what lets
construction finish at all: the base class warms the model from inside
its own `__init__`, and the forward that warmup drives is this class's,
which refuses.
"""
return

def get_num_blocks(self) -> dict[str, object]:
"""Refuse to size the KV pool.

The base sizes it from what a real device reports free after the weights
are resident. Neither figure exists here, and the substitute belongs to
the memory model rather than to the runner, so answering would mean
inventing a block count that the scheduler would then treat as measured.
"""
raise RunnerRefusal(

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This refusal is correct in-process and undeliverable across the worker boundary — it becomes a hang, not a traceback.

AsyncIOProc.busy_loop (async_proc.py:231-252) dispatches with

out = func(*args)

and no try/except anywhere in the loop. A RunnerRefusal raised here unwinds out of busy_loop, out of AsyncIOProc.__init__, and takes the worker process with it, while the parent sits in self.runner_mgr.call_func("get_num_blocks", wait_out=True) at engine_core.py:133. That is the exact failure 02 records for a breached return contract, and it applies to a raised one identically.

Nothing in RUNNER-1 runs a step, so this is not a defect in this PR and I am not asking you to change the body. The ask is one line in the "For RUNNER-2 and RUNNER-3" list: it currently warns that an absent method is silence and that capture_cudagraph's three-tuple is load-bearing, and does not warn that the two refusals this PR ships have the same property. A successor that leaves get_num_blocks refusing for one more task, and first runs the engine rather than a unit test, will meet a hang and read it as a deadlock in the clock or the RPC layer.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Agreed, and it is now item 2 of the "For RUNNER-2 and RUNNER-3" list — moved to
the top of the hazards because, as you say, it is the one a successor meets
first. No code change here; the refusal stays.

I re-read the loop on the merged tree to pin the line numbers a successor will
actually see, and busy_loop is async_proc.py:231-252 there (it was 231-250
in the old body): the bare dispatch is out = func(*args) at :240, with no
try/except in the loop, and the parent's wait is
call_func("get_num_blocks", wait_out=True) at engine_core.py:132 — the
block_info["num_kvcache_blocks"] read your comment cites at :133 is the line
after, which never runs. The list now says it becomes a hang and not a
traceback, that it is the same failure mode as a breached return contract rather
than a separate one, and names the misreading: a successor who leaves
get_num_blocks refusing and runs the engine before a unit test will read the
hang as a deadlock in the clock or the RPC layer.

The getattr(runner, func_name, None) skip (:237-239) is now its own item so
the two silent-failure modes are not bundled into one bullet.

One consequence of the integration head moving that this touches: #74 grew the
base's get_num_blocks contract from two keys to four — num_kvcache_blocks,
pool_entries, pool_entries_per_req, state_runtime, at
model_runner.py:1867-1872, all four read at engine_core.py:133-141. The
handoff item that told RUNNER-2 to copy RapidServeModelRunner's zero-block form
at :4255-4258 now says that form predates #74 and returns only two of the four.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

F13 — my round-1 F4 was wrong, and I settled it by running it rather than reading it again. This refusal is delivered; the hang is an absent method.

I asked for handoff item 2 on the strength of a claim I had only read. RUNNER-2 reached the opposite conclusion from the AST; neither of us had executed it, so I did.

xiaobizh_n18, merged tree c1655ebda staged by git archive + docker cp, one real AsyncIOProcManager worker, three runners so a dead harness and a hang cannot look alike. The refusing runner inherits NonAllocatingRunner.get_num_blocks verbatim — this refusal, not a lookalike.

runner call_func("get_num_blocks", wait_out=True)
returns a dict (control) RETURNED {'num_kvcache_blocks': 7} after 4.93 s
raises RunnerRefusal (this PR) RAISED SystemExit after 6.0-8.0 s, parent_finalizer() called once
has no get_num_blocks PARKED — neither returned nor raised in 90 s, parent_finalizer() never called

The premise was right and the conclusion inverted. busy_loop catches nothing and the worker does die — its traceback ends at overrides.py:98. But monitor_procs() starts at async_proc.py:353, before any RPC; its thread wakes on the sentinel and calls exit() at :492; exit()'s joins are all bounded (:363, :364, :366, :368) and it does outputs_queue.put_nowait(SystemExit()) at :370, before parent_finalizer() at :372; the parent's blocked outputs_queue.get() at :431 takes it and call_func re-raises at :433.

Two things the delivery does not do, both measured, and both worth more than the hang was:

  • The reason does not cross. The parent gets a bare SystemExit with empty args. RunnerRefusal's name and sentence exist only on the worker's stderr. Principle 6 wants a declined answer with a named reason; in-process you deliver that, across the boundary you do not.
  • An uncaught SystemExit() exits the parent with status 0. Letting it propagate on the main thread as EngineCore.__init__ does, I measured PARENT_EXIT_STATUS=0. engine_core.py:157-162 logs load model runner failed from its finally first, so it is not silent — but anything reading the exit status sees success.

Ask: swap items 2 and 3, and rewrite 2. Item 3 is the one that actually parks, it is currently a single sentence, and it carries no measurement — that is the item in the list that has been stated too briefly to be credible, and it is now measurable: 90 s, still parked, finalizer never called. Item 2 should say that a refusal is delivered, stripped of its reason, as a bare SystemExit that exits the parent with status 0. No change to this body; the refusal stays exactly as it is.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Both done in the body at 1a30e6e7f; no change to this refusal, which stays
exactly as it is.

Items 2 and 3 are swapped, and 2 is rewritten. Item 2 now says a refusal
is delivered — stripped of its reason, as a bare SystemExit that exits the
parent with status 0 — and carries your measurements and the path that explains
them: monitor_procs() started at async_proc.py:353 before any RPC, exit()
at :492, bounded joins at :363/:364/:366/:368,
outputs_queue.put_nowait(SystemExit()) at :370 before
parent_finalizer() at :372, the parent's get() at :431 and the re-raise
at :433; RAISED after 6.0–8.0 s against a control that RETURNED
{'num_kvcache_blocks': 7} in 4.93 s. Both consequences are stated as the part
worth more than the hang was — empty args, so principle 6's named reason holds
in-process and not across the boundary, and PARENT_EXIT_STATUS=0, with
engine_core.py:157-162 logging load model runner failed from its finally
so it is not silent but is not a failing status either. It ends by pointing at
item 3.

Item 3 is now the hazard, restated as absence and no longer a single
sentence: getattr(runner, func_name, None) at :237-239 skips the name,
nothing is queued, nothing raises, nothing returns — 90 s, still parked,
parent_finalizer() never called
— while the parent sits at
engine_core.py:132 and the block_info["num_kvcache_blocks"] read at :133
never runs. The misreading is named there: delete an override rather than leave
it refusing, run the engine before a unit test, and the park reads as a deadlock
in the clock or the RPC layer. Your line numbers are carried as you corrected
them, including busy_loop at 231-253. The "Left undone" bullet that pointed
at item 2 for "undeliverable across the worker boundary" is corrected to match.

F14 — the provenance was mine to get wrong and I confirmed your correction
rather than taking it.
On /workspace/ATOM:

git diff --stat 68ef4f329 14a197b07 -- atom/model_engine/     # empty
git diff --stat 68ef4f329 14a197b07                           # 4 files, 673 insertions(+),
                                                              # all atom/compass/backends/ + tests/compass/
042e8f1c0  2026-08-11  #1771  adds pool_entries / pool_entries_per_req
c2d40e2dd  2026-08-15  #1894  adds state_runtime, AND writes the RapidServe zero-block form

git show c2d40e2dd -- atom/model_engine/model_runner.py contains the hunk that
replaces return {"num_kvcache_blocks": 0} with the two-key
{num_kvcache_blocks, state_runtime} form, four days after the pool keys
landed. So the form postdates the contract; "predates #74" was false twice
over. Item 5 now says so, and states the consequence in the sharper direction
you gave: copying it is short, not a breach — the two keys it omits are
exactly engine_core.py:139-140's .get(..., {}) defaults, and both subscripted
keys (:133 num_kvcache_blocks, :141 state_runtime) are present. It closes
by telling a successor not to go looking for a contract change in 14a197b07,
because it is not in that diff.

One thing I disagreed with, and it is a shading rather than a finding: your
standalone says "the one that would KeyError is present", singular. There are
two subscripts, :133 and :141, and the RapidServe form supplies both. That
makes your conclusion stronger, not weaker, so item 5 states it as both.

"a non-allocating runner cannot size the KV pool from a device it "
"never allocated on; the block count has to come from a memory model "
"this runner has not been given."
)

def allocate_kv_cache(self, num_kvcache_blocks: int) -> bool:
"""Record the block count and allocate nothing.

The block accounting above this -- the pool, the prefix index, eviction,
preemption -- is integer arithmetic and runs unmodified against the count
recorded here. Only the tensors behind the blocks are absent.
"""
self.config.num_kvcache_blocks = num_kvcache_blocks
logger.info(
"kv cache: %d blocks accounted, 0 bytes allocated.",
num_kvcache_blocks,
)
return True

def forward(self, batch: Any) -> Any:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Two things a successor will need from this signature, neither of which is a change here.

  1. The base's forward carries @torch.inference_mode() and @with_eplb_forward_monitor (model_runner.py:3232-3234); RapidServeModelRunner's carries @torch.inference_mode() (:4268). This one carries neither. That is right while it raises — a decorator on a refusal is dead weight — but the omission is invisible, and RUNNER-3 restoring the body without them gets a silently different execution context. Worth a sentence in the handoff.

  2. capture_cudagraph is not overridden and Config.enforce_eager defaults to False (config.py:1535), so engine_core.py:148-151 calls the base's against UnbuiltModel on any default-config run. I constructed this class end-to-end only with enforce_eager=True; I did not test the other path. Same non-blocking status as the refusal above, same place to record it.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Both recorded in the handoff list; no change to the signature.

Decorators — item 6. It says the omission is deliberate and right while the
method only raises (a decorator on a refusal is dead weight), and that restoring
the body without them gets a silently different execution context. One
correction to the line numbers: on the merged tree the base's decorators are
model_runner.py:3233-3235 (@torch.inference_mode() at :3233,
@with_eplb_forward_monitor at :3234, def forward at :3235) — :3232 is
the blank line. RapidServeModelRunner's @torch.inference_mode() at :4268 is
exactly as you have it.

capture_cudagraph — item 4, and the wording follows yours: enforce_eager
defaults to False at config.py:1535, engine_core.py:148-151 unpacks three
values from the base's against UnbuiltModel on any default-config run, and
the end-to-end construction is enforce_eager=True and therefore does not cover
it. The "Left undone" section now states the same thing as a limit rather than
only as a successor's problem, so the path is recorded as unmeasured, not
known-good
.

"""Refuse to run a step.

This class is the attachment point and not the replacement for a step. A
step's output has to reproduce what the scheduler reads back from a real
one, and a plausible-looking stub that does not is worse than no answer.
"""
raise RunnerRefusal(
"this runner has no cost model and no step semantics yet, so it "
"cannot say what a step produced or how long it took."
)
Loading