Run the structural efficiency tests in CI - #808
Open
ZacharyZcR wants to merge 2 commits into
Open
Conversation
tests/test_inefficiency.py looked for `c/glm.exe`. The engine was renamed
glm -> colibri, and .exe only exists on Windows, so that path resolves on no
platform at all: _engine_present() has been returning False everywhere and
all 8 tests in the file have been skipping instead of running.
$ python3 -m unittest discover -s tests -p 'test_inefficiency.py' -v
... skipped 'glm.exe not built (run: make glm.exe)' x8
They are the throughput floor, the disk-wait share ceiling, the PROFILE
phase assertions and the CPU-vs-CUDA teacher-forcing agreement -- the
regression tests for the engine being fast, which is the property this
project exists for. A green `make test-python` has been reporting on none
of it.
tools/efficiency.py had the same default (`../glm.exe`), so anything else
driving it landed on the same missing file.
Both now resolve colibri/colibri.exe, preferring whichever exists. With the
engine built, the skip reason becomes the honest one:
... skipped 'glm_tiny fixture absent (... run tools/make_glm_oracle.py)'
which is a real prerequisite -- the fixture is gitignored and needs torch to
generate -- rather than a name nobody updated.
CONTRIBUTING's oracle command had the same stale binary and never said where
glm_tiny comes from, so a new contributor following it hits two dead ends in
one line.
The previous commit made tests/test_inefficiency.py resolve the engine again,
so those tests can run for the first time. Nothing runs them.
This adds a job that generates the glm_tiny fixture and runs the three that
assert STRUCTURE, in the same shape as inkling-oracle: build, generate,
verify the fixture with the token-exact oracle, then test against it.
test_telemetry_parses the tok/s and PROFILE lines the
whole efficiency tooling parses
test_profile_phases_present_and_nonneg no phase missing, none negative
(negative = double-counted)
test_disk_wait_not_dominant a fully-resident model must not
become I/O bound
Not the throughput floor. A shared runner's tok/s is not reproducible, and a
flaky perf gate gets muted within a week -- the floor stays a local/lab check.
test_cpu_vs_cpu_determinism is also left out, and that one is worth naming.
Despite the name, its second assertion is a tok/s stability bound (two runs
within 25%). On this box, two IDENTICAL greedy runs came out 38.8% apart --
the tiny replay is 15 ms end to end, so scheduler noise dominates it
completely. Its first assertion (hit-rate identical between runs) is a real
determinism check and does hold; splitting the two so the honest half can be
gated is worth a separate change, not a silent edit inside a CI commit.
Verified in both directions. The three pass three times in a row locally, and
breaking the REPLAY telemetry line on purpose (tok/s -> BROKEN) fails
test_telemetry_parses, so the gate is not vacuous.
ZacharyZcR
marked this pull request as ready for review
August 3, 2026 21:11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft. Stacked on #804 — that PR makes these tests resolve the engine at all; this one runs them.
Context
#804 fixed
tests/test_inefficiency.pylooking forc/glm.exe, a name that stopped existing at theglm→colibrirename. All 8 tests had been skipping on every platform. Now they can run — and nothing runs them.What this adds
A job in the shape of
inkling-oracle: build, generate theglm_tinyfixture, verify the fixture with the token-exact oracle first, then test against it.Three tests, all structural:
test_telemetry_parsestest_profile_phases_present_and_nonnegtest_disk_wait_not_dominantWhat is deliberately excluded, and why
The throughput floor (
test_tiny_tok_s_floor). A shared runner's tok/s is not reproducible. A flaky perf gate gets muted within a week and then protects nothing. That one stays a local / lab-machine check.test_cpu_vs_cpu_determinism— and this one is worth naming, because the exclusion looks arbitrary until you read it.Despite the name, it makes two assertions:
The second is a performance bound wearing a correctness name. Two identical greedy runs on a quiet local box came out 38.8% apart — the tiny replay is 15 ms end to end, so scheduler noise dominates it completely. On a shared runner it would be flaky on day one.
The first assertion is a genuine determinism check and it holds. Splitting the two, so the honest half can be gated, is worth doing — but as its own change, not silently inside a CI commit.
Verified both directions
A gate that cannot fail is worse than no gate:
tok/s→BROKEN) makestest_telemetry_parsesfailNote for anyone reproducing locally
If
python3 -m unittest tests.test_inefficiency...givesModuleNotFoundError, check for atestspackage in your user site-packages shadowing the repo's directory —PYTHONNOUSERSITE=1confirms it. That is a local-environment issue, not a repo one; CI is clean.