feat(infra): EXP-10 드래프터 fc 프로브 + 브래킷 자동화 + 트레이스 diff 모드 - #236
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed CLI/printing bugs that can crash the new tools in common error/usage paths (trace --diff parsing and bracket cmd_judge output on non-brackets).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds measurement-focused infrastructure for Kernel Campaign II: a new EXP-10 offline probe for the drafter FC GEMM, automated base→cand→base bracket recording/judging, and an enhanced trace step-composition tool with a diff mode—along with contract tests and runbook updates to codify the methodology.
Changes:
- Added
probes/drafter_fc_check.pyto benchmark drafter FC projection arms (stock/bf16/F.linear/mk_w4) under cold-weight conditions and report headroom vs a W4 DRAM bound. - Added
bench/bracket.pyto record bracket “legs” to jsonl and judge significance using base-pair drift as the floor, with env snapshot warnings and C≠1 exclusion. - Refactored
tools/trace_step_composition.pyintoanalyze()/report()and added a base-vs-cand diff report emphasizing kernel-count deltas as authoritative.
File summaries
| File | Description |
|---|---|
| tools/trace_step_composition.py | Adds reusable analysis + diff output for comparing two traces side-by-side. |
| tests/test_logic.py | Adds contract tests for bracket judging, trace composition diff, and the new EXP-10 probe. |
| RUNBOOK_KERNEL_CAMPAIGN2.md | Documents EXP-10 methodology and bracket tooling/runbook discipline. |
| probes/drafter_fc_check.py | New offline probe to time drafter FC GEMM alternatives and print a bound-based verdict. |
| bench/bracket.py | New CLI tool to record bracket legs and judge effects against reboot drift, with env snapshots. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| for s in rep["segments"]: | ||
| print(f" {s['tag']:<6} n={s['n']:<3} median {s['median']}") |
| def main(argv: list) -> int: | ||
| paths = [] | ||
| diff_path = None | ||
| i = 0 | ||
| while i < len(argv): | ||
| if argv[i] == "--diff": | ||
| diff_path = argv[i + 1] | ||
| i += 2 | ||
| else: | ||
| paths.append(argv[i]) | ||
| i += 1 | ||
| if not paths: | ||
| print(__doc__) | ||
| return 2 | ||
| base = analyze(paths[0]) | ||
| if diff_path: | ||
| diff(base, analyze(diff_path)) | ||
| else: | ||
| report(base) |
| if args.config: | ||
| c = json.load(open(args.config)) | ||
| c = c.get("text_config", c) |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| rows = [] | ||
| for m in args.ms: | ||
| x = (torch.randn(m, K, device=dev) * 1.5).to(torch.bfloat16) | ||
| ref = torch.mm(x.float(), ref_w) |
There was a problem hiding this comment.
Reference matmul uses untransposed weight
High Severity
ref multiplies activations by ref_w in [N, K] layout, but every timed arm treats w as an F.linear weight and applies x @ w.T. At the fleet shape this inner-dimension mismatch raises before any arm is scored, so the EXP-10 probe cannot produce a verdict.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f29a71b. Configure here.
| with torch.cuda.stream(st): | ||
| with torch.cuda.graph(g, stream=st): | ||
| for i in range(n): | ||
| fn_of_i(i) |
There was a problem hiding this comment.
Cold-weight graph timing is broken
High Severity
_graph_us calls fn_of_i(i) to cycle NW distinct weights, but every arm is a zero-argument closure over a single pack. Timing therefore raises TypeError immediately, and even a *args workaround would still replay one 42 MB tensor instead of the documented 252 MB cold set.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit f29a71b. Configure here.
| arms = [] | ||
|
|
||
| def arm_stock(): | ||
| return fp8_fp4_gemm(x, packs[0][0], packs[0][1]) |
There was a problem hiding this comment.
Stock arm skips pack layout fallback
Medium Severity
Both packed_sf layouts are packed, then the stock arm always uses packs[0]. Production keeps the first layout that passes a value check, because packing itself often succeeds for the wrong scale encoding. A fleet image that needs packed_sf=False then fails the e2m1 gate or the GEMM even though the other pack would work.
Reviewed by Cursor Bugbot for commit f29a71b. Configure here.
원장이 지명하고 아직 착수 없던 세 가지를 코드로: 1. probes/drafter_fc_check.py — 보충 분해 3 이 split-K 후보로 지명한 드래프터 fc 투영([M,5x4096]x[4096], 스텝당 1회, 809 us = fp4 42 MB 를 52 GB/s 로 읽는 것)을 실형상에서 잰다. 이미지에 이미 있는 네 팔(stock fp8xfp4 / bf16 mm / linear NT / mk_w4 베스트 에포트), 6 가중치 순환 그래프로 콜드 웨이트, 판정은 W4 스트림 바운드(220 us) 대비 몫. 베스트 팔이 바운드에 닿으면 이 축은 프로브 한 번으로 닫힌다. 2. bench/bracket.py leg|judge — base->cand->base 브래킷의 기록과 판정. 원장 규율을 코드로 박은 것: 판정 채널 C=1 step/s (tok/s / (1 + k x raw_acc)), 유의 문턱 = base 두 다리의 드리프트, 다리 사이 env 스냅샷 차이 경고(#116), C!=1 은 기록만 하고 판정에서 제외. 재기동은 사람이(자동화 읽기 전용). 3. tools/trace_step_composition.py --diff — 두 트레이스의 범주별 ms/개수 나란히. 개수 열이 정본 채널이라는 부트 스트랩 문구 포함. 단일 모드 출력은 불변. 게이트: tests/test_logic.py 2700 검사 전부 OK (신규 35: judge 판정 6 케이스, analyze 합성 트레이스 함수 테스트, 프로브 계약). 프로브는 srv4 이미지에서 --config 로 실형상 확인 후 첫 실측.
f29a71b to
e46a6eb
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f29a71bf7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| rows = [] | ||
| for m in args.ms: | ||
| x = (torch.randn(m, K, device=dev) * 1.5).to(torch.bfloat16) | ||
| ref = torch.mm(x.float(), ref_w) |
There was a problem hiding this comment.
At the default fleet shape, x.float() is [M, 20480] while ref_w is [4096, 20480], so this torch.mm raises an incompatible-shapes error before any benchmark arm runs. The reference must multiply by the transposed [20480, 4096] weight, as the bf16 and linear arms do.
Useful? React with 👍 / 👎.
| for i in range(3): | ||
| fn_of_i(i) |
There was a problem hiding this comment.
After the reference computation is fixed, every timing attempt still fails here because _graph_us calls fn_of_i(i), but all supplied arms (arm_stock, the bf16/linear lambdas, and arm_mk) accept zero arguments. Consequently the probe raises TypeError during warmup and cannot produce any timings; either provide genuinely indexed callables and cycled weights or invoke the zero-argument closures without i.
Useful? React with 👍 / 👎.
| def chan(p): | ||
| return p["step_s"] if p.get("step_s") is not None else p["tok_s"] |
There was a problem hiding this comment.
When /metrics is temporarily unavailable or the exact acceptance counters are absent, step_s is None and this silently substitutes tok_s. That allows a single bracket to compare normalized step/s reps against unnormalized token/s reps, even though speculative acceptance directly changes token throughput, and the judge header still claims the channel is step/s. Such reps should invalidate the leg rather than participate in the verdict.
Useful? React with 👍 / 👎.
| r = json.loads(line) | ||
| if args.name in (None, r.get("name")): | ||
| records.append(r) |
There was a problem hiding this comment.
If the default append-only output already contains a completed bracket with the same experiment name, this filter loads both the old and new runs. judge then uses the earliest first base and latest final base to compute drift and treats every intervening candidate as part of one bracket, producing verdicts from unrelated boots. Select a specific run/bracket identifier or restrict processing to the latest complete base→cand→base sequence.
Useful? React with 👍 / 👎.
| base = analyze(paths[0]) | ||
| if diff_path: | ||
| diff(base, analyze(diff_path)) | ||
| else: | ||
| report(base) |
There was a problem hiding this comment.
For the documented invocation trace_step_composition.py base.gz cand.gz, the parser puts both files in paths but leaves diff_path unset, so this branch reports only base.gz and silently ignores the candidate. Either map paths[1] to the candidate or reject that syntax and remove it from the usage text.
Useful? React with 👍 / 👎.
| base = analyze(paths[0]) | ||
| if diff_path: | ||
| diff(base, analyze(diff_path)) | ||
| else: | ||
| report(base) |
There was a problem hiding this comment.
For the documented invocation trace_step_composition.py base.gz cand.gz, the parser puts both files in paths but leaves diff_path unset, so this branch reports only base.gz and silently ignores the candidate. Either map paths[1] to the candidate or reject that syntax and remove it from the usage text.
Useful? React with 👍 / 👎.
| try: | ||
| p4 = mk.build_mk_weight_w4(w) | ||
| arm_mk = lambda: mk._gemm_call(x, p4, N) # noqa: E731 | ||
| r = _rel(arm_mk(), ref) |
There was a problem hiding this comment.
On the documented probe invocation the megakernel knobs are unset, so importing glm53_megakernel leaves its lazy _EXT as None; this direct _gemm_call therefore raises when it dereferences _EXT.run_gemm. The surrounding catch misleadingly reports that the lane refused K=20480, meaning mk_w4 is always skipped without actually testing its shape support unless some unrelated initialization happened first. Initialize the extension explicitly before probing, as the existing megakernel benchmark does.
Useful? React with 👍 / 👎.
| def arm_stock(): | ||
| return fp8_fp4_gemm(x, packs[0][0], packs[0][1]) |
There was a problem hiding this comment.
The preceding loop retains every pack whose quantization call succeeds, but production cannot determine the valid scale layout from packing success alone—it tries both layouts through the GEMM and keeps the one that passes its value check. Always using packs[0] can therefore fail or report garbage on an image where packed_sf=False is the compatible layout even though that valid pack is already in packs; exercise each pack through the stock kernel and select the passing one.
Useful? React with 👍 / 👎.
| for s in rep["segments"]: | ||
| print(f" {s['tag']:<6} n={s['n']:<3} median {s['median']}") |
There was a problem hiding this comment.
When judge rejects an incomplete sequence such as base→cand, it returns segment entries containing only tag and n, but this unconditional lookup of s['median'] raises KeyError. Thus the CLI prints the bracket warning and then crashes instead of cleanly returning its intended invalid-bracket status; either populate medians on the early-return path or make this formatter handle summary-only segments.
Useful? React with 👍 / 👎.
| rec = {"ts": datetime.datetime.now().isoformat(timespec="seconds"), | ||
| "name": args.name, "tag": args.tag, "conc": args.conc, | ||
| "num_spec": args.num_spec, "git": _git_sha(), | ||
| "env": {k: os.environ[k] for k in ENV_KEYS}, | ||
| "model": bd.MODEL, "reps": []} |
There was a problem hiding this comment.
This snapshot reads the environment of the later bracket.py leg process, not the environment with which the server was booted. In the runbook's documented flow, knobs are supplied as one-command prefixes such as VLLM_GLM53_ASYNC_DFLASH=1 bash launchers/...; those assignments expire when the launcher returns, and the shown leg commands do not re-export them, so base and candidate records normally both contain an empty or identical snapshot. Require the boot configuration as explicit leg metadata or document and enforce passing it again, otherwise the advertised knob-delivery audit cannot detect a misconfigured boot.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e46a6eb. Configure here.
| for p in rep["problems"]: | ||
| print(f" !! {p}") | ||
| for s in rep["segments"]: | ||
| print(f" {s['tag']:<6} n={s['n']:<3} median {s['median']}") |
There was a problem hiding this comment.
Judge crashes on incomplete brackets
Medium Severity
When records are not base→cand→base, judge returns segments that only have tag and n. cmd_judge then reads s['median'] and raises KeyError, so the CLI dies on the exact case it is supposed to diagnose.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e46a6eb. Configure here.
무엇 (부팅 0번으로 여는 세 가지)
1.
probes/drafter_fc_check.py— EXP-10보충 분해 3 이 "split-K 후보"로 지명해놓고 프로브가 없던 자리: 드래프터 fc 투영
[M, K=5×4096] × [4096], 스텝당 1회, 현재 809 µs = fp4 42 MB 를 52~104 GB/s 로 읽는 것(W4 스트림 하한190 GB/s 의 1/41/2)._fp8_fp4_dense_gemm/ bf16 mm(168 MB eager 경로) / F.linear NT / mk_w4 베스트 에포트(K=4096 전용이면 SKIP 이 답)--config로 플릿 실형상 확인, 발사 비트동일 검사2.
bench/bracket.py leg|judge— 브래킷 자동화부팅 CV 1.3
1.7% 때문에 잠긴 0.51% 큐(EXP-4, KPOOL_FUSED_TOPK, 헤드 AllGather)를 여는 측정 인프라. 원장 규율을 코드로:tok/s ÷ (1 + k×raw_acc))3.
tools/trace_step_composition.py --diff base cand붙여서 계속 쓰던 그 표의 2-트레이스 비교. 개수 열이 정본이라는 부트스트랩 문구 포함. 단일 모드 출력 불변.
게이트
tests/test_logic.py2700 검사 전부 OK (신규 35)다음 (이 PR 머지 후)
srv4 에서
drafter_fc_check.py --config <draft config>첫 실측 → 숫자가 커널 제작을 결정. a_ready 체인 융합 확장은 그 다음 원장 지명 축.Note
Low Risk
Benchmark/probe scripts and runbook only; no production inference or auth paths touched.
Overview
Adds measurement-only tooling for Kernel Campaign II: offline GPU probes, scripted A/B bracket recording/judging, and trace comparison—no serving-path or kernel changes in this PR.
EXP-10 introduces
probes/drafter_fc_check.pyto time the drafter fc GEMM at fleet shape (K=5×4096) across stock fp8×fp4, bf16torch.mm/F.linear, and megakernel W4, using cold-weight CUDA graphs and a built-in verdict vs the W4 DRAM bound. The runbook documents EXP-10 and when to usebench/bracket.py.bench/bracket.pyimplementsleg(append reps toruns/bracket.jsonlviabench-dec, C=1 step/s, env knob snapshots) andjudge(base→cand→base medians, significance vs base-pair drift, flags env mismatches; C≠1 legs recorded but excluded).tools/trace_step_composition.pysplits printing intoanalyze()/report()and adds--diffside-by-side output where per-category kernel-count deltas are labeled as ground truth vs ms deltas.Contract tests in
tests/test_logic.pycoverjudge()outcomes, syntheticanalyze()/diffbehavior, and probe source contracts.Reviewed by Cursor Bugbot for commit e46a6eb. Bugbot is set up for automated code reviews on this repo. Configure here.