offload-matrix: Tier 0 + Tier 1 test suite, and fix a partial-cache blind spot - #825
Closed
noonghunna wants to merge 5 commits into
Closed
offload-matrix: Tier 0 + Tier 1 test suite, and fix a partial-cache blind spot#825noonghunna wants to merge 5 commits into
noonghunna wants to merge 5 commits into
Conversation
The harness produces numbers that decide serving configs, so its failure mode is a plausible WRONG number, not a crash. Six defects of exactly that shape shipped during its development and were caught by luck (#824). These eight checks guard them, run in ~1.2 s, and need no server, no GPU and no model. 1 syntax, both halves 2 MODEL is mandatory, refusal is actionable 3 COLUMN PARITY -- header width == emit_row values + status, and the width must be derived from $HDR. Guards the printf that carried 32 specifiers for 33 arguments: bash re-ran the format for the surplus arg, so every arm wrote a short row PLUS a junk status-only row. 4 STATUS-ENUM PARITY, sweep <-> renderer. This one has already caught real drift: CACHE_DISABLED shipped with no renderer annotation within an hour of being added. Mutation-tested -- deleting that annotation reds the suite. 5 pairing key includes `shape`. Workload shape FLIPS THE SIGN of speculative results, so a mispair manufactures a gain that does not exist. 6 engine-scope guard refuses a non-llama.cpp binary 7 PLAN=1 enumerates arms and boots nothing 8 renderer survives a mixed TSV (OK + BOOT_FAIL + CACHE_DISABLED) and lists non-OK arms separately instead of folding them into conclusions Check 7 carries a minimal fake llama-server stub -- it exists to pass the scope probe in check 6, and it is the seed the Tier 1 mocked-server suite grows from. Tier 1 (mocked server, ~25 fake arms) and Tier 2 (real short run) still to come; Tier 2 stays out of the default scripts/tests sweep.
The sweep mkdir's $PWD/offload-matrix-out unless OUT_DIR is set, so running the Tier 0 suite from the repo root left an artifact directory behind -- including a results TSV from the refusal-path checks. Every invocation now writes into the test's temp dir, and check 9 asserts the repo is clean afterwards: a test that leaves artifacts is a bad test, and this one would have leaked on every CI run.
…suite The CACHE_DISABLED detector keyed on the presence of a "[moe-cache] enabled:" line. A PER-DEVICE budget failure still prints that line while one GPU runs with no pool at all -- measured live 2026-07-30: CUDA0 got no budget, CUDA1 allocated 69 slots, "enabled:" printed, and the arm reported OK. Throughput on such a run is a blend of a cached and an uncached device and is not comparable with either. Detection now counts DEVICES THAT ACTUALLY GOT A POOL against the device count: 0 pooled -> CACHE_DISABLED (as before) 0 < pooled < NGPU -> CACHE_PARTIAL (new) Tier 1 suite runs the real sweep end-to-end against a fake llama-server that emits the exact log lines the script scrapes and serves the two endpoints it calls -- no GPU, no model, no engine, ~15 s. Five scenarios, each asserting a status plus the row invariants: healthy -> OK partial -> CACHE_PARTIAL (the regression guard for the fix above) nobudget -> CACHE_DISABLED, remedy named bypass -> INVALID_BYPASS notokens -> NO_TOKENS plus a renderer pass proving every failure status is surfaced and separated from the conclusions. Throughput VALUES are deliberately never asserted -- they are not portable, and the failure mode that matters is a healthy status on an unhealthy run. The fixture documents the scrape contract by construction: it emits only what the script actually reads, so an accidental dependency on some other log line fails the suite instead of working by luck.
… shell The EXIT trap used a process-name pattern that matches the killing command's own command line. It self-kills, the shell dies with exit 144, the trap never finishes, and the fixture it was meant to reap survives -- observed exactly that, with a fake server still bound to a test port after the suite reported ok. Bracketing the first character of the pattern makes it unable to match itself. Same class as the rig-wide pgrep/pkill -f footgun. Sharp edge worth knowing: this also fires from OUTSIDE the suite. Any shell whose command line happens to contain the unbracketed pattern -- for instance a commit message describing this very bug -- becomes a match and gets killed by the trap of a concurrently running test. Hence this message avoids spelling it out.
… callers Both suites shell out to python3, so the repo's locale invariant applies to them (#779): python decodes reads, stdout and argv with the LOCALE codec unless UTF-8 mode is on, and a single-byte locale corrupts quietly rather than raising. test-locale-utf8 caught this before the PR opened, which is what it is for.
Owner
Author
|
Superseded by #835, which grew this into the 3-tier suite (static + mocked + tier2-real, both fake-binary fixtures) and folded the partial-cache blind spot into the per-device CACHE_DISABLED fix (#824). Every file here landed there in evolved form. Closing to keep the queue honest — the test coverage this PR pioneered is on master. |
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
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.
Closes the first two tiers of #824, and fixes a detector defect found while validating the engine work that motivated it.
The fix:
CACHE_PARTIALCACHE_DISABLEDkeyed on the presence of a[moe-cache] enabled:line. A per-device budget failure still prints that line while one GPU runs with no pool at all — measured live on the reference rig: CUDA0 got no budget, CUDA1 allocated a 69-slot pool,enabled:printed, and the arm reportedOK. Throughput on such a run blends a cached and an uncached device and is comparable to neither.Detection now counts devices that actually got a pool against the device count:
CACHE_DISABLED(unchanged)CACHE_PARTIAL(new)Tier 0 — static, 9 checks, ~1.2 s
No server, no GPU, no model. Each check guards a defect that actually shipped in this harness during development (all six are listed in #824):
emit_rowvalues + status, and the width must derive from$HDR. Guards theprintfthat carried 32 specifiers for 33 arguments: bash re-ran the format for the surplus arg, so every arm wrote a short row plus a junk status-only row.CACHE_DISABLEDshipped with no renderer annotation within an hour of being added, and it caughtCACHE_PARTIALin this very PR.shape— workload shape flips the sign of speculative results, so a mispair manufactures a gain that does not exist.MODELmandatory, engine-scope refusal,PLAN=1boots nothing, renderer survives a mixed TSV, and the suite leaves no artifacts in the repo.Mutation-tested: deleting the
CACHE_DISABLEDannotation reds the suite with the right message. A green run is not evidence that a check can fail.Tier 1 — mocked server, 5 scenarios, ~15 s
Runs the real sweep end-to-end against a fake
llama-serverthat emits the exact log lines the script scrapes and serves the two endpoints it calls. No GPU, no model, no engine — but boot → drive → scrape → emit → render all execute, which is where the silent defects live.OKCACHE_PARTIAL— the regression guard for the fix aboveCACHE_DISABLED, remedy namedINVALID_BYPASSNO_TOKENSEach case also asserts row invariants (field count == header count;
OKimplies non-zero throughput;bypass>0implies notOK), and a final pass proves the renderer surfaces every failure status and separates them from the conclusions.Throughput values are deliberately never asserted. They are not portable across rigs, and the failure mode that matters is a healthy status on an unhealthy run.
The fixture documents the scrape contract by construction — it emits only what the script actually reads, so an accidental dependency on some other log line fails the suite rather than working by luck.
Notes
test-locale-utf8caught the missingPYTHONUTF8export in both suites before this PR opened — exactly its job.scripts/tests/*.shsweep so the suite remains fast.Tests
test-offload-matrix✅ ·test-offload-matrix-mocked✅ ·test-locale-utf8✅ (93 scripts) ·test-artifact-inventory✅. Scoped change — no registry, compose or profile touched.