moe_timing: route on device in the campaign's replay, and gate on it - #112
Open
qichao-arlo-wang wants to merge 4 commits into
Open
moe_timing: route on device in the campaign's replay, and gate on it#112qichao-arlo-wang wants to merge 4 commits into
qichao-arlo-wang wants to merge 4 commits into
Conversation
…on it `qwen3_trace_replay` read `topk_indices` out of the trace and preloaded them into INT SRAM, so V_TOPK -- the instruction the routed-MoE ISA work exists for -- never appeared in a program the timing campaign measured. `router_topk` was in MOE_STAGES with nothing emitting it, and every cycle count the campaign produced excluded selection. The trace cannot supply logits: `generate_true_routing_with_weights` takes topk and softmax of the real router's output and keeps only those. `router_logits` rebuilds a BF16 row that reproduces them -- log-weights in the recorded order, a strict one-ulp ladder where BF16 flattens a tie (uniform timing-only weights are entirely ties, and V_TOPK breaks ties by low index), an unselected floor, and the whole row shifted positive so the zero padding `prestage_bf16_vram_matrix` writes can never outrank a real expert. It refuses, naming the token, when the recorded order and the recorded weights cannot both hold. The gate this enables is the point. The existing zero-input smoke gate compares an all-zero accumulator against an all-zero golden, which holds for any routing at all -- injecting three router faults (per-token INT base dropped, every token selected from row 0, router not emitted) fails the new gate on all three and the smoke gate on none. `qwen3_trace_replay` had no CI job because every real trace needs a 30B forward pass. `synthetic_trace` draws Gaussian logits and goes through the same `trace_from_record` and schema validation, so the program is now built and run in CI. Verified at MLEN 128 and at MLEN 64, where 128 experts span two rows per token. Still excluded, and now said so in the manifest rather than only in a comment: the hidden->num_experts GEMM that produced the logits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The gate was raising and nothing downstream asked about it. Two places pick a run's verdict out of the results JSON and both read only `zero_input_smoke_gate`, which compares an all-zero accumulator against an all-zero golden -- true for any routing at all, so it cannot fail on a routing fault: - `_prior_run_passed` decides what `--skip-existing` re-runs. Its own docstring is the argument: the results file is written before the gate assertion, so a failed run leaves a complete-looking artifact. A router failure was therefore permanent -- the trace read as done and was never retried. - `summarize_run` produces `functional_gate_passed`, which feeds `export_selected`'s filter and `_exit_code`'s tally. A run that routed to the wrong experts was averaged into the medians and counted as passing. Both now AND the router gate in. Absent stays passing: artifacts written before routing moved on device carry no `router_gate`, and calling those failures would re-run every run in an existing out_root. Injecting the strict reading breaks two pre-existing guards, which is what makes that choice load-bearing rather than lenient. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… SRAM ceiling Ten findings from a review of this branch. `pytest transactional_emulator/testbench/moe_timing/` passed on main and failed at collection here: the campaign guard installs a fake `torch` at module scope for the pytest-only CI job, and this branch adds the first tests under `qwen/` that need the real one -- `campaign/` sorts first, so the stub won. `_stub` now substitutes only where the real package is unimportable, verified both ways: 47 passed on the directory, and 27 passed on a venv holding pytest and nothing else, which is what that CI job installs. `reconstruct_router_logits` silently required weights summing to 1, and nothing in the schema or the validator asks for that. Weights from a softmax over all experts without renormalising -- norm_topk_prob=false -- came back scaled by 1/sum and were refused as "the recorded order and the recorded weights disagree", a true statement about the arithmetic and a wrong diagnosis of the trace. Checked explicitly now, naming the sum and the cause. The token count is bounded by scalar SRAM at 94 for Qwen3's top_k of 8. That already held -- the compiler's FPRAM allocator raises -- but as `FPRAM overflow: need 128 at addr 897`, which names neither the trace nor the tokens. Checked before the two token-sized allocations, reading the base from an already-allocated variable so it cannot drift: both refuse at exactly 94, and the message says what would fit. The new recipe argument is documented against it and now threads one `--mlen` to the trace and the run rather than relying on two independent defaults agreeing. `_router_gate` recomputed the reconstruction to build its expectation instead of using the tensor that was staged, so the staging path was unchecked by the gate that exists to compare the device against it. It now takes the staged rows; `max_weight_error` is unchanged to the digit. Four guards drive the gate from dumps written by hand, covering the offsets, the per-token stride and the transposed reshape that every previous guard passed through. Also: a router failure with no gate from the precedence chain reported unknown rather than failed; an empty `router_gate` made a run re-run forever; the gate's tolerance was an undocumented duplicate of the reconstruction's; and a comment put BF16's mantissa at 8 bits and its ulp at 2^-8, contradicting the one below it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ps, fixture drift The strongest finding was one level up from the last round's. `test_router_logits` and `test_qwen3_router_replay` reached CI only through the string `just test-router-logits` inside one workflow step, and the existing wiring guard exempts anything that imports torch -- so deleting that line would have retired 25 guards with every check green. `test_every_guard_is_reachable_from_ci` now requires each guard file to be named by a job or by a recipe a job invokes. It matches on a boundary that excludes hyphens: `\b` puts one between `builder` and `-renamed`, so the first version passed when an invocation was renamed and the recipe was not -- caught by injecting exactly that. It also found a guard that had never run: `test-sliced-layer-builder` was in the justfile and in no job. It passes; it is wired in now. `_router_gate` decoded the SRAM dumps without checking they were there. A missing dump raised FileNotFoundError from inside the gate and a truncated one died in reshape, both scoring the run as a crash and naming numpy. It now reports a gate that could not be read, failing closed and saying which file and by how much -- the run happened and cannot be verified, which is neither passing nor the "absent means unknown" case that applies to artifacts predating the gate. `_check_scalar_sram_capacity` assumed route_fp_scratch was the last FPRAM allocation. True today, and the router GEMM this replay defers allocates its own, which would silently shorten the token limit. The assumption is now asserted against the allocator's high-water after compile, with a message naming the check to update. The gate guards wrote their dumps and their manifest at a hand-typed base of 145, so any FP layout change would move the real base while they kept agreeing with themselves -- the failure the campaign guards were rewritten to avoid. They now take the manifest and the staged logits from the module's real build. Also: teardown drops modules imported against the stubs, not just the stub names; the per-token allclose in _verify is one vectorised comparison; the manifest records whether the run's mlen matched the trace's, now that mlen decides the logit fold; and the workflow names the guards and the replay as separate steps. 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.
qwen3_trace_replayis the only decoder-level program the timing campaign measures. It readtopk_indicesout of the trace and preloaded them into INT SRAM, soV_TOPKnever appeared in a measured program.router_topkwas a name inMOE_STAGESthat nothing emitted, and every cycle count the campaign produced excluded selection.V_TOPKin the measured programrouter_topkin the stage profileThe trace has no logits
generate_true_routing_with_weightsruns the real router, takestopk+softmax, and keeps only those. Regenerating logits means a Qwen3-30B-A3B forward pass, which is what replaying a trace exists to avoid.router_logitsrebuilds a BF16 row that reproduces the recorded top-k instead:loginverts softmax up to the additive constant the shift suppliesbuild_route_tracessubstitutes uniform weights for timing-only runs, which is entirely ties, and V_TOPK breaks ties by low index — so[9, 2, 5]would come back[2, 5, 9]without itprestage_bf16_vram_matrixpads with zeros and raw log-weights are all ≤ 0, so a row laid out one column short would have a zero pad outrank every real expert, silently and with plausible numbersWhat it does not rebuild: the 120 unselected logits get a floor, not their true values. Selection and weighting are exercised; the tail's spread is not.
The gate is the point
The existing gate compares an all-zero accumulator against an all-zero golden — which holds for any routing, because the expert weights are dummy zeros. Injecting three router faults:
maintodayExpert ids are integers, so that half of the gate has no tolerance in it. The weights are checked against softmax over the BF16 logits actually staged, not against the trace's — folding the reconstruction's own bound in here as well would test the reconstruction twice and the device not at all.
First CI coverage for this program
It had none: every real trace needs the 30B checkpoint.
synthetic_tracedraws Gaussian logits and hands them to the sametrace_from_recordand the same schema validation a captured trace goes through, so the shape is real even though the routing distribution is not. Gaussian rather than uniform on purpose — it puts near-ties in the top-k tail, which is where the BF16 reconstruction is hardest.Verification
expert_ids_match: trueon both; max weight error 3.3e-4just test-timing-gatesstill exits 0, all four gates passruff format --check/ruff checkclean across 103 files; the pytest-only CI job 23 passedtest_every_guard_file_is_wired_into_cicaught that the new test file was torch-free and therefore being pulled into the pytest-only job, where it cannot run. That was a real hole rather than a guard to work around: theMLEN 64fold had no coverage at all, so the guards that close it are the ones that make the torch dependency honest.Not in scope
The router GEMM. The trace kept the top-k, not the logits, so
hidden -> num_expertscannot be replayed faithfully — emitting it with dummy weights would produce arbitrary selection and destroy the trace's routing distribution, which is the whole point of trace replay.router_gemm_included: falseis now in the manifest, and the routing-tax note inexport_pilot_resultssays so instead of claiming top-k is excluded too.🤖 Generated with Claude Code