refactor(telemetry): one expert history for every engine (#700) - #716
Merged
Conversation
5 tasks
terrizoaguimor
force-pushed
the
route-telemetry
branch
from
July 31, 2026 21:52
f5af90d to
c485cdf
Compare
.coli_usage had two incompatible writers and two engines that could not
produce it at all: colibri.c wrote sparse text, inkling.c wrote a dense
IKU1 binary block, kimi_k3.c and olmoe.c wrote nothing, and both writers
defaulted to the same filename. A model directory shared between two
engines ended up holding a history the other one refuses.
route_trace.h now owns the format and the ROUTE_TRACE stream, with no
Model or Cfg dependency, so any engine can write and read the same bytes.
colibri.c and telemetry.h move onto it; the duplicated writer and
pin_load duplicated parser are gone.
The header records are encoded as triples with a negative layer so that
readers built before this change parse them, discard them, and continue
into the data:
-1 <n_layers> <n_experts>
-2 <format_version> <engine_id>
That forces three rules, each of which is asserted in the tests against a
literal copy of the pre-existing reader loop rather than against
expectations of it:
- every field must be numeric, or fscanf returns < 3 and the reader
silently drops every record after that point;
- the engine id must be the THIRD field, because old readers parse the
second with %d and glm_moe_dsa hashes to 3815245270, above INT_MAX;
- exactly three fields, never more. A fourth leaves one value
unconsumed, which desynchronises an old reader and makes it fabricate
an admitted record from that leftover plus the start of the next line.
A history whose identity does not match is refused, by name, on every path
the engine chooses for itself: the .coli_usage next to the weights,
PIN=auto, and the AUTOPIN seed. A path the user typed is honoured and says
so once, because pins decide which bytes are resident and never what the
model computes, so the worst case is self-inflicted and visible in the hit
rate. Format version and IKU1 geometry are parse correctness rather than
attribution and are never relaxed.
An empty history stays a zero-byte file, since PIN=auto tests file size to
decide whether to fall back to stats.txt.
rt_drop_row releases the counter row of a layer that does not route, which
is the load admission rule and not an optimisation: every reader treats a
NULL row as "no expert here to attribute a count to", the shape eusage had
before, and which the Vulkan expert-selection path also relies on.
Verified against the unmodified tree: byte-identical ROUTE_TRACE dumps,
history data, [STATS] line, [PIN] placement and dashboard EMAP, plus an
unmodified binary reading a new-format file and this build reading a
legacy one. Confirmed on GLM-5.2 (78x256) as well as the small fixture,
and the unit test passes cross-compiled with MinGW-w64 under wine, where
the CRT rename() shim in compat.h turns out to be load-bearing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
terrizoaguimor
force-pushed
the
route-telemetry
branch
from
July 31, 2026 22:22
78e5de6 to
b956472
Compare
This was referenced Aug 1, 2026
terrizoaguimor
added a commit
to terrizoaguimor/colibri
that referenced
this pull request
Aug 1, 2026
Third engine onto the shared telemetry from JustVugg#700, after colibri.c (JustVugg#716) and kimi_k3.c (JustVugg#719). The counters, the load and the save move to route_trace.h; the bump sites are untouched, since m->eusage now aliases rt_counts_all(). The transition is the point: rt_load reads both the IKU1 block every previous inkling wrote and the shared text format, so an existing .coli_usage keeps working and is rewritten in the new form. The header checks the old pins_load did inline -- magic, n_layers, n_experts against the live config -- all still fire, now in the reader, which additionally refuses a history written by a different engine. Narrower than before, never wider; and parse geometry does not bend to a trusted path the way identity does. Fixes a latent bug on the way: pins_load documents PIN_N=0 as "seeds the ranking from the history but pins nothing", but the npin guard ran before the memcpy, so the counters started at zero and the next usage_save replaced the accumulated ranking with that run's counts alone. Verified on the tiny fixture: rebuild clean, oracle token-exact (36/36 teacher-forced, 24/24 generated), make check 273 passed / 34 skipped, and the built engine reading a planted legacy history pins the right 9 experts while dropping the dense layer's counts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
terrizoaguimor
added a commit
to terrizoaguimor/colibri
that referenced
this pull request
Aug 1, 2026
Fourth and final engine for JustVugg#700, after colibri.c (JustVugg#716), kimi_k3.c (JustVugg#719) and inkling.c (JustVugg#742). Every engine that routes experts now writes the same format. m->freq moves from one flat [n_layers * n_experts] block to route_trace.h's row per layer, so the three read sites -- the hot-pin ranking, the bump in moe() and the LFRU eviction guard -- change shape but not meaning. last_access stays flat; it is a separate array route_trace.h does not own. Every olmoe layer routes and there is no MTP layer, so the only row dropped is the spare one rt_init leaves. The history is opt-in behind COLI_USAGE, exactly as kimi_k3 does it: with the variable unset nothing is loaded and nothing is written, so the default path is unchanged. PPL=1 deliberately does not save, so a loss sweep cannot fold its own tokens into the persisted ranking. Verified against the real OLMoE-1B-7B-0125-Instruct converted with tools/convert_olmoe_merged.py. Output is byte-identical to dev on the same run, down to hit=1216 miss=832 -- the eviction path decides exactly as before, which is the property the layout change could have broken. The COLI_USAGE round trip writes the -1/-2 header with the olmoe identity, leaves no row for the dropped layer, and accumulates 2048 -> 4096 across two runs. make check green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
terrizoaguimor
added a commit
to terrizoaguimor/colibri
that referenced
this pull request
Aug 1, 2026
Fourth and final engine for JustVugg#700, after colibri.c (JustVugg#716), kimi_k3.c (JustVugg#719) and inkling.c (JustVugg#742). Every engine that routes experts now writes the same format. m->freq moves from one flat [n_layers * n_experts] block to route_trace.h's row per layer, so the three read sites -- the hot-pin ranking, the bump in moe() and the LFRU eviction guard -- change shape but not meaning. last_access stays flat; it is a separate array route_trace.h does not own. Every olmoe layer routes and there is no MTP layer, so the only row dropped is the spare one rt_init leaves. The history is opt-in behind COLI_USAGE, exactly as kimi_k3 does it: with the variable unset nothing is loaded and nothing is written, so the default path is unchanged. PPL=1 deliberately does not save, so a loss sweep cannot fold its own tokens into the persisted ranking. Verified against the real OLMoE-1B-7B-0125-Instruct converted with tools/convert_olmoe_merged.py. Output is byte-identical to dev on the same run, down to hit=1216 miss=832 -- the eviction path decides exactly as before, which is the property the layout change could have broken. The COLI_USAGE round trip writes the -1/-2 header with the olmoe identity, leaves no row for the dropped layer, and accumulates 2048 -> 4096 across two runs. make check green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 format half of #700, approved in
this comment.
.coli_usagehad two incompatible writers and two engines that could not produce it at all:colibri.cwrote sparse text,inkling.cwrote a denseIKU1binary block,kimi_k3.candolmoe.cwrote nothing, and both writers defaulted to the same filename — so a model directoryshared between two engines ended up holding a history the other one refuses.
c/route_trace.hnow owns the history format and theROUTE_TRACEstream with noModelorCfgdependency.colibri.candtelemetry.hmove onto it; the duplicated writer andpin_load's duplicated parser are gone.No new environment variables, and nothing new is printed on a healthy run.
The header records
Encoded as triples with a negative layer so that readers built before this change parse
them, discard them, and continue into the data — both are a
while(fscanf(f,"%d %d %u",...)==3)guarded byl>=0. That buys backward compatibilitywithout a second code path, and it forces three rules. Each is asserted in
tests/test_route_trace.cagainst a literal copy of the old reader loop, rather thanagainst my expectations of it:
fscanfreturns < 3 and the reader silently drops every record after that point%d, andglm_moe_dsahashes to 3815245270 — aboveINT_MAXAn empty history stays a zero-byte file:
PIN=autodecides usability by file size and fallsback to
stats.txt, so a header written into an empty history would quietly cost that fallback.Refusing a foreign history
A history whose identity does not match is refused by name on every path the engine chooses
for itself — the
.coli_usagebeside the weights,PIN=auto, and theAUTOPINseed:A path the user typed is honoured, and says so once:
Trust follows how the path was reached, not what the file contains, which is why
pin_loadtakes it as an argument — one function serves both kinds of path. Unlike a mismatched weight
container (#529), a mismatched history cannot change what the model computes: pins decide which
bytes are resident, so the worst case is slower, self-inflicted and visible in the hit rate.
Format version and
IKU1geometry are parse correctness rather than attribution and are neverrelaxed.
rt_drop_rowreleases the counter row of a layer that does not route. This is the loadadmission rule, not an optimisation — every reader treats a NULL row as "no expert here to
attribute a count to", which is the shape
eusagealready had, and which the Vulkanexpert-selection path depends on too.
Verification
Measured against the unmodified tree at
292ed4c, not argued:ROUTE_TRACEdump, before vs after[STATS]line[PIN] placementEMAPlinemake check, both builds warning-freetests/test_route_traceThe Windows run is worth one line: it failed 11 assertions first, because the CRT
rename()fails when the destination exists — the reason
compat.h's shim exists in the first place. Theheader includes
compat.hwith that reason inline, so the facility depends on the C libraryplus that one shared platform header, and no engine types.
docs/routing-telemetry.mddocuments the format, why each field sits where it does, thethree-field rule with a worked example of the desynchronisation, and how a new engine adopts it.
Linked from
docs/tuning.mdbeside the learning-cache section.Not in this PR
kimi_k3.cadoption follows as its own PR now that #676 has landed (five call sites, compilingclean against the merged file).
olmoe.cand teachinginkling.c's reader the text layout comeafter that.
inklingcurrently declines a new-format file cleanly, which is correct until then.🤖 Generated with Claude Code