Skip to content

compass(spec): the refusal two readings earn, and a width table nobody entered #146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jgong5 merged 1 commit into from
Sep 23, 2026
Merged

compass(spec): the refusal two readings earn, and a width table nobody entered #146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 14 additions & 1 deletion atom/compass/spec/memory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,16 @@
Four checks, then, narrowing: whether one rank's readings describe a card at
all, whether the cache that rank built was sized by its own budget, the ranks
against each other, and the one number that survives them. A contaminated run
can fail more than one, and the one raised is the first in that order.
can fail more than one, and **each check is asked of every rank before the next

@jgong5 jgong5 Sep 22, 2026

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.

Finding 3 (non-blocking): this sentence is a little wider than the code.

"each check is asked of every rank before the next check is asked" is exactly true of the first two checks — and that is the whole of the fix, so the claim it supports is right. It is loose about the last two: the spread and the absolute limit are asked of the ranks collectively, not of every rank, because across_ranks reduces with min/max before either fires.

I checked that this does not weaken the guarantee. Running every permutation of rank sets that earn three and four of the checks at once, the head gives one outcome per set where the parent gave two, and the aggregate pair was never order-dependent to begin with — the spread refusal names no rank at all. So the order the paragraph states is the order the code executes, verified end to end; only the phrase generalises past what the last two checks do.

No change asked for. Recorded so a reader who takes the sentence literally and goes looking for a per-rank loop behind checks three and four knows why there isn't one.

check is asked**, so the one raised is the first in that order. Asking them the
other way round -- both questions of rank 0, then both of rank 1 -- would make
the refusal a function of the order the ranks were listed in: two readings, one
that is no card and one whose cache a neighbour sized, earn a different refusal
depending on which of them is rank 0. The two name different things to repair,
so a run sent after the wrong one can fix what it was told to fix and see the
same refusal again. The rank a refusal names is still the first that failed the
check that fired, which is a question about which reading came in where and has
no other answer.

One difference from the engine's own arithmetic is deliberate: the engine floors
this term at zero and this does not. A floor turns an impossible reading into a
Expand Down Expand Up @@ -204,6 +213,10 @@ def non_torch_across_ranks(
"out plausible whether or not the readings themselves can be "
"true together",
)
# Every rank is asked the first question before any is asked the second, so
# which of the two a run is refused by is decided by the readings and not by
# the position a failing rank happened to arrive in.
for rank, reading in enumerate(readings):
if reading.free_was_binding:
raise SpecRefusal(
Rule.DEVICE_WIDE,
Expand Down
11 changes: 9 additions & 2 deletions atom/compass/spec/validate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,16 @@
#: about -- and a question registered as reading a field it cannot speak for
#: reports a run that could not ask it and a run that asked and found nothing
#: as the same record. Derived from `FILLED_BY`, so a probe given the width
#: that has none empties this with no second list to remember.
#: that has none empties this with no second list to remember. A table no
#: probe is named for at all contributes nothing here rather than failing the
#: derivation: this runs while the module is being imported, so a subscript
#: would answer a table nobody has entered yet by denying every caller of the
#: package, including the ones with no interest in probes, and the reader would
#: be told which import failed rather than which term is missing. The naming is
#: left to `probe_for`, which refuses such a term by name, and the two lists
#: are held against each other by a test.
PROBE_TABLES = tuple(
path for path in WIDTH_TABLES if None in FILLED_BY[path.rsplit(".", 1)[-1]]
path for path in WIDTH_TABLES if None in FILLED_BY.get(path.rsplit(".", 1)[-1], ())

@jgong5 jgong5 Sep 22, 2026

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.

Accepted, with one measurement for the record — the default swallows a mistyped key as well as a missing one, and the test is what catches it.

The review brief for this PR asked whether .get(name, ()) could weaken the derivation by answering a misspelt FILLED_BY key the same way it answers an absent one. It can, in the derivation itself: with the key mistyped against an otherwise-correct schema path, the head imports cleanly, the table quietly leaves PROBE_TABLES, and validate(document, tp_widths=(1,)) returns ok=True with the probe question silent — where the parent's subscript raised at import.

It does not go unnoticed in the package. Measured at this head, with the key mistyped:

probe_for("allocator_retained_after_load_bytes", 1)
  -> REFUSED: `allocator_retained_after_load_bytes` is not one of the constants measured per tensor-parallel width
set(FILLED_BY) == {leaf of p for p in WIDTH_TABLES}  ->  False   (the guard fires)

So both halves of what this comment promises are true: probe_for names the term at the call site, and test_the_probe_table_names_every_constant_the_schema_keys_by_width catches the mistype from either direction — an extra key as readily as a missing one. The comment is accurate as written and I am asking for no change.

Two things for whoever next touches this area, neither introduced here: the guard is a test rather than an instrument in the package, and it compares leaf names rather than full paths, so two width tables sharing a leaf under different blocks would pass it.

)
STACK_PINS = tuple(f"{PINNED_BLOCK}.{component}" for component in PINNED)

Expand Down
108 changes: 107 additions & 1 deletion tests/compass/test_spec_verbs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"""

import copy
import importlib.util

import pytest

Expand All @@ -48,7 +49,9 @@
tokenizer_fragment,
validate,
)
from atom.compass.spec.fields import BY_PATH, Kind
from atom.compass.spec import fields as schema_module
from atom.compass.spec import probes as probes_module
from atom.compass.spec.fields import BY_PATH, Field, Kind
from atom.compass.spec.probes import FILLED_BY, cpu_counts
from atom.compass.spec.tokenizers import ENTRY_FIELDS
from atom.compass.spec.validate import (
Expand Down Expand Up @@ -1408,6 +1411,40 @@ def test_a_rank_whose_free_memory_was_binding_is_refused_and_named():
assert "92100000000.0" in refused.value.what


def test_which_refusal_a_pair_of_readings_earns_is_not_decided_by_rank_order():
# One rank read numbers no card produced; the other built its cache out of
# the gap a neighbour left. Each fails a different one of the two per-rank
# checks, so the pair earns both -- and the two name different things to
# repair: a reading to take again, against a card to take it on. Every rank
# is asked the first question before any is asked the second, so the pair
# earns the same refusal whichever of them arrived first. Asked rank by
# rank instead, whichever was listed first would decide the remedy, and a
# run sent after the wrong one repairs it and is refused again.
binding = reading(200.0e9)
impossible = DeviceMemory(
free_bytes=400.0e9,
total_bytes=CAPACITY,
reserved_bytes=-120.0e9,
kv_budget_bytes=1.0,
)
assert binding.free_was_binding and not binding.impossible
assert impossible.impossible and not impossible.free_was_binding
for ranks, first_to_fail in (
([binding, impossible], "rank 1"),
([impossible, binding], "rank 0"),
):
with pytest.raises(SpecRefusal) as refused:
non_torch_across_ranks(2, ranks, 7.2e9)
assert refused.value.rule is Rule.DEVICE_WIDE
assert "which is not a reading of a card" in refused.value.what
assert "set the cache size" not in refused.value.what
assert "take them again and find out" in refused.value.remedy
# Which check fired is the readings'; the rank it names is still the
# first that failed that check, which is a question about where in the
# sequence the reading came in and has no other answer.
assert first_to_fail in refused.value.what


def test_a_reading_the_engine_would_floor_at_zero_is_refused_here():
# The engine floors this term at zero. A probe does not: a floor turns an
# impossible reading into a plausible one.
Expand Down Expand Up @@ -1625,6 +1662,75 @@ def test_the_probe_question_reads_only_the_tables_a_probe_can_fall_short_on():
}


def reimported_validate(name):
"""`validate` imported again, the way a session imports it the first time.

The probe tables are derived once, while the module is being imported, so a
test about that derivation has to import the module rather than call
something in it. This loads a second instance out of the same file under a
name of its own, and does not register it, so the instance the rest of the
suite is holding is the one it started with.
"""
location = importlib.util.find_spec("atom.compass.spec.validate").origin
loaded = importlib.util.spec_from_file_location(
f"atom.compass.spec.{name}", location
)
module = importlib.util.module_from_spec(loaded)
loaded.loader.exec_module(module)
return module


def test_a_width_table_no_probe_is_named_for_is_refused_and_not_an_import_error(
monkeypatch,
):
# A width-keyed constant written into the schema and not into the probe
# table is the ordinary shape of a half-finished change, and the two lists
# are held together by a test for exactly that reason. The derivation runs
# while the module is being imported, so a table it could not answer for
# would deny every caller of the package instead -- and that test would
# fail at collection, as an import error naming the test session rather
# than the term nobody entered.
added = Field("device.runtime_constants.graph_replay_pool_bytes", Kind.WIDTH_TABLE)
monkeypatch.setattr(schema_module, "SCHEMA", schema_module.SCHEMA + (added,))
under_test = reimported_validate("validate_with_a_table_no_probe_is_named_for")
assert added.path in under_test.WIDTH_TABLES
# A table with no entry has no hole to report, so it is not a probe table.
assert added.path not in under_test.PROBE_TABLES
# The term is named where a caller asks about it, by the refusal this
# package exists to give.
with pytest.raises(SpecRefusal) as refused:
probe_for("graph_replay_pool_bytes", 1)
assert refused.value.rule is Rule.NO_DEFAULTS
assert "`graph_replay_pool_bytes` is not one of the constants" in refused.value.what
# And what the test holding the two lists together now sees is its own
# comparison, failing on the term that is missing from the table.
assert set(FILLED_BY) != {
path.rsplit(".", 1)[-1] for path in under_test.WIDTH_TABLES
}


def test_a_probe_given_the_width_that_has_none_empties_the_probe_tables(monkeypatch):
# The property the derivation exists for, and the one a second list would
# lose: give the hole a probe and the question has nothing left to ask
# about, with nothing else to edit. The run still counts it as asked -- it
# is not a question this run could not reach -- and it finds nothing to say.
monkeypatch.setitem(
FILLED_BY,
"allocator_retained_after_load_bytes",
(probes_module.SINGLE_CARD, probes_module.MULTI_RANK),
)
under_test = reimported_validate("validate_with_every_width_filled")
assert under_test.PROBE_TABLES == ()
checked = under_test.validate(merged().document, tp_widths=(16,))

@jgong5 jgong5 Sep 22, 2026

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.

Finding 2 (non-blocking): this pins one of the two widths the PR body reports.

The body states the property at both tp_widths=(1,) and tp_widths=(16,); the test asserts it at (16,) only. (1,) is the more interesting of the two — it is the ok=True case, where "asked the question and found nothing to say" and "could not ask the question" are hardest to tell apart, which is exactly the inversion this derivation exists to keep closed.

I measured it at the head rather than asking for it:

PROBE_TABLES = ()
validate(document, tp_widths=(1,))  -> ok=True   PROBES in not_asked: []  in asked_in_part: []
validate(document, tp_widths=(16,)) -> ok=False  PROBES in not_asked: []  in asked_in_part: []

Both hold, so this is coverage rather than a defect, and one extra validate(...) line would close it if you happen to be touching the file. Not worth a round 2 on its own.

assert not checked.ok
assert not any(
condition.startswith(PROBES_ASKED) for condition in checked.not_asked
)
assert not any(
condition.startswith(PROBES_ASKED) for condition in checked.asked_in_part
)


def test_the_probe_question_says_it_could_not_be_asked_when_its_table_is_gone():
# The other width table still resolves, so the width question was asked of
# part of what it reads. The probe question was not asked at all, and the
Expand Down