Repository navigation
compass(gates): locate aiter for gate_gpu.sh without importing it - #271
Conversation
gate_gpu.sh read aiter's checkout from aiter.__file__, so recording the aiter version imported aiter, whose architecture probe shells out to rocminfo and hangs uninterruptibly on a wedged driver. The checkout is now located by the artifact store's module_root, which resolves the top-level name with importlib.util.find_spec and executes nothing. The git describe call and the BASE_AITER comparison are unchanged. The new test runs the gate's own resolution lines against stub aiter and torch packages that raise when executed, and fails if the import is restored. Closes #166 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| # version is most worth recording. module_root resolves a top-level name with | ||
| # importlib.util.find_spec, which executes nothing, and its import chain loads | ||
| # neither aiter nor torch. | ||
| AITER_DIR=$(python -c 'from atom.compass.artifacts.provenance import module_root; print(module_root("aiter"))' 2>/dev/null) |
There was a problem hiding this comment.
Reuse ruling (principles 3, 4 and 2): accepted. Keep the atom import. Do not inline it. Not blocking.
- The gate already depends on
import atombeing cheap. Line 148 callscompass_require_tree "$ROOT", which runspython -c 'import atom; ...'under the samePYTHONPATH=$ROOT, ten lines before this one. This line only extends an existing dependency into theatom.compass.artifactspackage; it adds no new kind of dependency. - One locator, not two (principle 4). The provenance stanza and this gate record the same aiter version. Because they share
module_root, they cannot disagree about which checkout that version came from. An inlinedfind_specone-liner would be a second definition that could drift from the first. - Measured on the merged tree
2114ab788, inxiaobizh_n18_cpu:from atom.compass.artifacts.provenance import module_roottook 37.7 ms, loaded 146 modules, and loaded none of torch, aiter, numpy, zmq or triton.- Bare
import atomtook 16.2 ms and loaded 104 modules, also none of them.
- The torch stub does catch a regression on this chain. Each mutation below appends
; import torch # noqato an existing line, so the line count is unchanged:
| mutation | lines | test_aiters_version_is_read_without_executing_aiter_or_torch |
test_the_located_name_is_top_level |
|---|---|---|---|
atom/compass/artifacts/rules.py:23 |
47→47 | FAILED, assert ['torch'] == [] |
passed |
atom/sampling_params.py:4, via atom/__init__ |
30→30 | FAILED, assert ['torch'] == [] |
passed |
rules.py:23 gets an aiter import that is swallowed: exec('try: import aiter / except BaseException: pass') |
47→47 | FAILED, assert ['aiter'] == [] |
passed |
The last row matters. The marker file catches an import that a try block hides, and a raise-only stub would miss it.
Residual, pre-existing and not introduced here (principle 6). 2>/dev/null still turns any failure of this line into AITER=UNKNOWN. That includes a future broken import in atom.compass.artifacts. UNKNOWN is treated as a mismatch, so the run warns loudly, but the warning reads "toolchain differs" rather than naming the lookup failure. This was the same before the PR, so I am not asking for a change here.
| """From the `AITER_DIR=` assignment through the `fi` that closes its branch.""" | ||
| lines = GATE_GPU.read_text().splitlines() | ||
| start = next(i for i, line in enumerate(lines) if line.startswith("AITER_DIR=")) | ||
| end = lines.index("fi", start) |
There was a problem hiding this comment.
Extraction ruling (principles 6 and 5): robust, and it fails loudly, but the failures do not say why. Not blocking.
It is not a line-drift guard. The extraction is anchored on content, not position. Every run was on node 18, with atom.__file__ asserted under each copy's root:
edit to gate_gpu.sh |
lines | result |
|---|---|---|
comment word: wedged → stuck (N1) |
326→326 | 2 passed |
extra comment line inserted above AITER_DIR=, so every line after it shifts (N2) |
326→327 | 2 passed |
trailing comment on the closing fi: fi # end (N3, no effect on behaviour) |
326→326 | test 1 FAILED: ValueError: too many values to unpack (expected 2) |
AITER_DIR renamed to AITER_ROOT (M7) |
326→326 | both FAILED: bare StopIteration |
if/fi collapsed into one AITER=$(git -C ...) line (M8) |
326→325 | test 1 FAILED: ValueError: too many values to unpack |
The test never passes silently when the markers move or vanish. But three of these failures name nothing:
- N3 changes no behaviour and still reddens.
lines.index("fi", start)needs a barefi. When the closingficarries a comment, the search runs on to the next barefi, which closes the toolchain-WARNING block. The script then prints thetorch:/baseline:lines, and the unpack fails. - M7 reports a bare
StopIteration.
Suggestion: have _resolution_lines check its own boundaries, with messages that say what went wrong. For example, assert start is not None, "gate_gpu.sh has no top-level AITER_DIR= line". Then check that the extracted block contains exactly one AITER= assignment, so a run-on is refused by name rather than by the unpack.
Scope limit, measured (M6c). Adding AITER_PY=$(python -c 'import aiter' 2>/dev/null) on its own line after the fi leaves 2 passed. The test holds the resolution lines, which is what it claims; it does not hold the whole script. A cheap whole-file text check would close that gap, for example no import aiter anywhere in gate_gpu.sh. That is optional: it is outside the named result.
|
This review was written by an agent. Before reviewing, it read the eight Design principles in Verdict: APPROVE at head What I checked1. The named result reproduces (principle 8). I ran the mutations in Node ids:
M3b isolates the behavioural test. With the text pin (B) still satisfied, a parent import is caught by A alone, through the marker file. M5 shows why the marker matters: a stub that only raises would miss an import wrapped in 2. Reuse via The deciding fact is that Measured on node 18:
3. The extraction markers (principle 6): robust, and loud, but the failures are unnamed. The details are inline on the test's
In every case it fails; it never passes silently. My suggestion is named asserts. That is not blocking.
4. No design-doc references (rules). The diff adds none. I grepped both files for 5. Size. 71 lines against a 20–40 estimate, which is inside the 2x stop. Most of it is the stub setup, which is what makes the mechanism testable. Rulings on the two surprisesGPU-tier trigger for
Watch item for the next GPU wave: on node 18, Stale README line: yes, file one follow-up. Widen it, because this PR makes more text stale than the README. Measured on node 18: bare
This PR also falsifies text that is outside its diff, so it cannot be commented inline:
This is one small documentation issue covering all five sites (principle 8). It is not a reason to widen this PR's file set. One piece of context for the named result, not a finding. Gate: the tree that will land
The gate printed:
This matches the developer's 5122 for the branch. Their control at |
Closes #166
What changed
scripts/compass/gate_gpu.shlocated aiter's checkout fromaiter.__file__, so recording the aiter version imported aiter. That import runs aiter's architecture probe, which shells out torocminfoand hangs uninterruptibly on a wedged driver.Only the locating step changes. It now calls the artifact store's
module_root("aiter"), which resolves the top-level name withimportlib.util.find_specand executes nothing. Thegit describe --tags --always --dirtycall and theBASE_AITERcomparison are unchanged.scripts/compass/gate_gpu.sh(production)tests/compass/test_gate_gpu_aiter_version.py(test)The total is 71 lines against a 20-40 estimate. That is inside the 2x stop, and most of it is the test's stub-package setup.
Decision: reuse ART-1's helper instead of duplicating it
The gate calls
atom.compass.artifacts.provenance.module_rootand has no second copy of the lookup. Reuse is only safe if importing that module does not itself reach aiter or the driver, so I measured that inxiaobizh_n18_cpu, withPYTHONPATHset to the staged branch root:from atom.compass.artifacts.provenance import module_roottook 38.3 ms and added 73 modules. None of them istorch,aiter,numpy,zmqortriton. Theatommodules it loads areatom,atom.sampling_params,atom.plugin{,.prepare,.sglang,.sglang.prepare}andatom.compass.artifacts.*, all standard-library only.atom/__init__.pyalready defersLLMEngineto first attribute access.module_root("aiter")returned/app/aiter-test/aiter, and afterwardsaiterwas not insys.modules.The test enforces this condition rather than taking it for granted. Its stub
torchalso raises, so a future eager import on this chain fails the test.Named result
No module is executed. The test cuts the gate's own lines out of
gate_gpu.sh, fromAITER_DIR=to the closingfi, and runs them withbash.PYTHONPATHis<stub site>:<repo>, where the stub site is a committed, tagged git checkout that holds anaiter/__init__.pyand atorch/__init__.py. Each stub writes a marker file and raises when executed. The test asserts three things:AITER_DIRis the stub's package directory;AITERequalsgit describe --tags --always --dirtyrun on that directory.It compares against that command's output, never a version literal, because two aiter versions are in circulation.
The test catches a restored import. I ran a line-count-preserving mutation and a null control in
xiaobizh_n18_cpu. Each ran against its own copy of the staged tree,gate_gpu.shstayed at 326 lines in every copy, andatom.__file__was printed under each copy's root:gate_gpu.shAITER_DIR=$(python -c 'import aiter, os; print(os.path.dirname(aiter.__file__))' 2>/dev/null)wedged driverchanged tostuck driverThe mutation fails these two tests:
tests/compass/test_gate_gpu_aiter_version.py::test_aiters_version_is_read_without_executing_aiter_or_torch, withAssertionError: a stub was executed/assert ['aiter'] == []. This is the behavioural failure: the old path executed the stub aiter and read UNKNOWN.tests/compass/test_gate_gpu_aiter_version.py::test_the_located_name_is_top_level, where'module_root("aiter")'no longer appears in the resolution lines.The same string on a healthy box. I ran a read-only check in node 18's GPU container
xiaobizh_n18. It ran only the resolution snippet from a staged copy of this branch; it did not run the GPU gate and took no GPU. The staged copy was removed afterwards. Results:module_root)/app/aiter-test/aiterv0.1.21.dev0-49-gf4e7c7509import aiter)/app/aiter-test/aiterv0.1.21.dev0-49-gf4e7c7509The two versions match each other and
BASE_AITER.Gates
Gate 1: ATOM's suite, unmodified. Measured on node 18,
xiaobizh_n18_cpu, using the tree's ownscripts/compass/gate_cpu.sh. Both trees weregit archivestages with.compass-commitand.compass-changedstamps, andatom.__file__was asserted under each staged root.ff9617f30(tip)1b8d1e3cfGATE_CPU_RC.compass-changedstamp)GATE_CPU_RCis not 98.gate_gpu.shis not a GPU-tier trigger, because the trigger list namesatom/modules, so the GPU tier did not fire and was not run.git merge-tree --write-tree ff9617f30 1b8d1e3cfgives2114ab788302bc26e719cf499538211618ea0ff5with rc 0. The branch is based on the current tip.Gate 2: CPU-only tests. The new test file passes, and
ruff check,ruff format --checkandblack --checkare clean on it.Gate 3: the named result, shown above.
Gate 4: independent review. Pending; the coordinator dispatches it.
Surprises
scripts/compass/README.mdstill says that on a wedged node ATOM's import hangs. That is no longer true ofimport atom:LLMEngineis now lazy, and the chain used here loads neither torch nor aiter. I did not edit the README, because it is outside this file set.gpu_gate_triggers.txtnamesatom/modules, notscripts/compass/, so the CPU gate says "gpu: not required" for this change. The only real-aiter evidence is the read-only node-18 check above.Left undone
gpu_dockercontainer, as the brief directs, so there is no direct demonstration on a wedged driver. The raising stub is the stand-in: it proves nothing is executed, which is the property that decides whether a hang can happen.scripts/compass/README.mdstatement aboutimport atomhanging is left for a follow-up.🤖 Generated with Claude Code