From 80a508dffcd8344fcce084ee6976246e6c918212 Mon Sep 17 00:00:00 2001 From: root Date: Tue, 22 Sep 2026 03:37:56 +0000 Subject: [PATCH] compass(spec): the refusal two readings earn, and a width table nobody entered The two per-rank memory checks were asked rank by rank, so a pair of readings that earns both of them earned whichever one the rank listed first happened to fail. The two name different things to repair -- a reading to take again, against a card to take it on -- so a run could be sent after the neighbour when what was actually wrong is that a rank did not read a card, fix that, and be refused again. Each check is now asked of every rank before the next check is asked, which is what the module already said it did: the refusal is decided by the readings and not by the order they arrived in, and the rank it names is still the first that failed the check that fired. The width tables the probe question can speak for are derived from the probe table while the module is being imported, and the derivation subscripted it. A width-keyed constant written into the schema and not into the probe table -- the ordinary shape of a half-finished change, and the one the test holding the two lists together exists to catch -- therefore raised at import and took the whole package with it, including every caller with no interest in probes. The test failed at collection, as an import error naming the test session rather than the term nobody entered. The derivation now reads the table with a default of no probes: a table with no entry has no hole to report, so it is not a probe table, and the term is named where a caller asks about it, by the refusal this package gives everything else it does not know. The property the derivation exists for is unchanged and is now tested from both sides: give the width that has no probe a probe and the probe tables empty, the question has nothing left to ask about, and there is no second list to edit. Co-Authored-By: Claude Opus 5 (1M context) --- atom/compass/spec/memory.py | 15 ++++- atom/compass/spec/validate.py | 11 +++- tests/compass/test_spec_verbs.py | 108 ++++++++++++++++++++++++++++++- 3 files changed, 130 insertions(+), 4 deletions(-) diff --git a/atom/compass/spec/memory.py b/atom/compass/spec/memory.py index e56743e78a..85430fdef8 100644 --- a/atom/compass/spec/memory.py +++ b/atom/compass/spec/memory.py @@ -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 +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 @@ -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, diff --git a/atom/compass/spec/validate.py b/atom/compass/spec/validate.py index cdd085d1b0..264898aa10 100644 --- a/atom/compass/spec/validate.py +++ b/atom/compass/spec/validate.py @@ -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], ()) ) STACK_PINS = tuple(f"{PINNED_BLOCK}.{component}" for component in PINNED) diff --git a/tests/compass/test_spec_verbs.py b/tests/compass/test_spec_verbs.py index 40233bd841..0d52a064bb 100644 --- a/tests/compass/test_spec_verbs.py +++ b/tests/compass/test_spec_verbs.py @@ -28,6 +28,7 @@ """ import copy +import importlib.util import pytest @@ -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 ( @@ -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. @@ -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,)) + 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