Skip to content

fix(trace): keep per-tuner failure signal in the GEMM-tuning audit row - #1212

Draft
iraj465 wants to merge 4 commits into
mainfrom
fix/gemm-tuning-trace-keeps-failures
Draft

fix(trace): keep per-tuner failure signal in the GEMM-tuning audit row#1212
iraj465 wants to merge 4 commits into
mainfrom
fix/gemm-tuning-trace-keeps-failures

Conversation

@iraj465

@iraj465 iraj465 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Branch: fix/gemm-tuning-trace-keeps-failuresmain
Base: origin/main = 25d914c
Commit: 45af0b8
Closes: #1210

NOTE:
In last few Hyperloom CI runs (14thAug-19thAug), there is little evidence that GEAK gemm tuning
integrate result into Hyperloom does have downstream issues, so parking it for
now to reduce last minute regressions due to low testing.

What was wrong

Forge Gemm-tuning has drop in effect on integration when multiple proposals of gemm-tuning through Hyperloom.

_trace_gemm_tuning_run wrote three fields per tuner — tuner,
best_micro_speedup, kept — and dropped status, elapsed_s, error and
error_class. The envelope carried no error_class even when a tuner named
one. A crashed tuner and a tuner that ran and found nothing produced the same
row.

38 of the 337 tuners_run rows in /shared_nfs/hyperloom-claw/ are failed or
empty_output. The trace showed none of them. Worked examples and the full
breakdown are in the issue.

What this changes

_TRACE_ERROR_CHARS = 400
_TRACE_TUNER_KEEP_NULL = frozenset({"tuner", "best_micro_speedup", "kept"})

Each row now carries status, elapsed_s, error_class and a truncated
error. Keys whose value is None are dropped, except the three the schema
already always emits — so a clean run's row is today's row plus status and
elapsed_s, and nothing else grows.

The envelope gains "error_class": result.get("error_class") or tuner_error_class,
so a per-tuner class is promoted only when the envelope has none of its own.

Generic by construction: no tuner name, no error string, and no exit code is
special-cased. The row records whatever the tuner reported.

Verification 1 — the patched function against real result.json

Executed the patched _trace_gemm_tuning_run against four unmodified
result.json from the campaign, and compared its output with the row currently
on disk.

workspace tuner status elapsed_s error_class
/shared_nfs/hyperloom-claw/Qwen3-8B/20260813T091428Z/runs/gemm_tuning/kernel_entry_gemm_tuning a8w8 failed (exit -9) 500.42 subprocess_error, promoted to the envelope
/shared_nfs/hyperloom-claw/DeepSeek-V4-Pro/20260815T002915Z/runs/gemm_tuning/kernel_entry_gemm_tuning fmoe_ck failed (exit 1) 288.54 subprocess_error
/shared_nfs/hyperloom-claw/Llama-3.1-8B-Instruct/20260813T091520Z/runs/gemm_tuning/kernel_entry_gemm_tuning sglang_dense_bf16 no_improvement 11.19
/shared_nfs/hyperloom-claw/Qwen3-8B/20260730T122359Z/runs/gemm_tuning/kernel_entry_gemm_tuning sglang_dense_bf16 ok, 15.6349x 1584.94

Rows 3 and 4 are the contrast the trace could not previously express: an
11-second run and a 1585-second run, both recorded today as an unremarkable
kept: null.

Verification 2 — tests

Five new unit tests in
src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py,
with a _gemm_trace_rows(tmp_path, result) helper that reads back the JSONL the
function actually wrote:

test pins
test_gemm_trace_keeps_tuner_failure status, elapsed, error_class and truncated error survive
test_gemm_trace_promotes_tuner_error_class_to_envelope promotion when the envelope has none
test_gemm_trace_envelope_error_class_wins_over_tuner the envelope's own class is never overwritten
test_gemm_trace_clean_run_row_stays_compact a successful run does not grow
test_gemm_trace_survives_non_mapping_tuner_entries a malformed tuners_run entry does not raise

The 7 request-handler / gemm test files, same interpreter, same machine:

main        : 433 passed, 110 failed
this branch : 438 passed, 110 failed

The 110 failures are pre-existing and the failure sets are byte-identical on
both sides (diff of the two FAILED lists is empty). The full repository
suite does not complete on this machine — it exceeds a 2-minute budget and hits
collection errors in test_multinode_server_log_dir.py and
test_gpu_probe_remote.py, identically before and after — so the run is scoped
to the files this change touches.

Related

This change makes a second defect visible rather than fixing it: 82 of the 96
sglang_dense_bf16 tuner runs are rejected by argparse
(gemm_tuner.py: error: unrecognized arguments: --libtype hipblaslt) and
recorded as no_improvement. Its root cause is in the standalone
forge_gemm_tune package, which is not in this repository. Filed separately.

That second defect is #1211.

@iraj465
iraj465 requested a review from a team as a code owner August 17, 2026 11:38
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown

CI E2E report — ✅ Succeeded

item value
result ✅ Succeeded
model Qwen/Qwen3-0.6B (dense)
resources 1× GPU, TP=1
PR branch fix/gemm-tuning-trace-keeps-failures
commit 363a8ba710a48301c4fa4366199c33cc719fe40a
session_id 5f0877c8-eaff-4d77-b35e-a4fed7b5f066
queue → dispatch 0s
run time 192m 8s
total 192m 8s

details

@lishuoshuo-amd

Copy link
Copy Markdown
Contributor

One correctness nit worth fixing, plus two comment inaccuracies. None of them block.

The envelope promotion is broader than its own comment claims

# The envelope leaves ``error_class`` unset even when every tuner failed;
# promote the first tuner's class so a failed run is greppable.
"error_class": result.get("error_class") or tuner_error_class,

The comment says "even when every tuner failed", but the code promotes whenever any tuner carries a class, regardless of the run's own status. In forge_gemm_tune's build_report, the has_candidate branch is evaluated before all_failed:

elif has_candidate:
    status = "ok"
    micro_decision = "candidate"
elif all_failed:
    status = "failed"

So a run where one tuner produced a candidate and another died lands on status=ok / micro_decision=candidate, which _run_forge_gemm_tuning then maps to decision="KEEP". That successful run now gets stamped error_class: subprocess_error in the audit row. Before this change the field was correctly absent; after it, it's actively wrong — the mirror image of the bug being fixed.
Narrow in practice: it needs a multi-tuner run with partial failure, and the #1210 census is 337 tuners_run rows across 315 result.json (~1.07 tuners/run), so single-tuner runs — which can't trigger it, since a lone failed tuner makes all_failed true and the promotion correct — dominate. Still cheap to close:

envelope_failed = str(result.get("status") or "").strip().lower() == "failed"
...
"error_class": result.get("error_class") or (tuner_error_class if envelope_failed else None),

None is already dropped by the row-level filter below, so nothing else changes. Worth a sixth test: status=ok + one failed tuner, assert the envelope carries no error_class. That's the one shape the current five don't cover.

kept has no producer

#: Audit keys emitted even when null. The first three predate the failure fields
#: and existing readers index on them; ...

kept is never written by anything. TuneResult.to_dict() in forge_gemm_tune has no such key, and on the Hyperloom side _validate_forge_gemm_tuning_e2e writes result["e2e_results"] = {"kept": ..., "reverted": ...} — a separate list, not a write-back into tuners_run[i]. It also runs strictly after _trace_gemm_tuning_run, which fires inside run_gemm_tuning_handler before it returns.
That's the real reason kept is null on all 337 rows, rather than #1210's "its absence would be indistinguishable from false". Keeping the key for backward compatibility is fine, but "existing readers index on them" isn't true of a field that is structurally always null. Might be worth saying so in the comment, or filing the write-back as a separate gap.

Two comment details

@iraj465

iraj465 commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

All three are fixed. HEAD is now c79a429f9.

The envelope promotion

Fixed, exactly as you wrote it. request_handlers.py:4895:

envelope_failed = str(result.get("status") or "").strip().lower() == "failed"
...
"error_class": result.get("error_class") or (tuner_error_class if envelope_failed else None),

Sixth test added:
test_gemm_trace_does_not_stamp_a_successful_run_with_a_tuner_error
status=ok with one failed tuner, asserts no error_class on the row. Six
gemm_trace tests pass.

You were right about the ordering in build_report. has_candidate is
evaluated before all_failed, so a partial-failure multi-tuner run lands on
status=ok and would have been stamped. Narrow, but wrong in the same
direction as the bug being fixed.

kept

Corrected. The comment no longer claims readers index on its value, and it now
names the producer that almost writes it:

#: path writes it -- re-measured 18 Aug 2026, it is null in all 366 tuner
#: entries across the 322 traces on record -- so no reader can be indexing on
#: its value, only on its presence. The write-back that would populate it,
#: ``_validate_forge_gemm_tuning_e2e``, writes a separate ``e2e_results`` list
#: and runs after this trace fires; that gap is real and is not fixed here.

I re-measured rather than reuse either number, because the campaign is live:
366 tuner entries across 322 traces, 0 non-null. Your 337-across-315 and my
earlier 364-across-320 are both correct for when they were taken.

The two comment details

Both corrected.

  • The _TRACE_ERROR_CHARS rationale no longer claims the truncated error
    identifies an argparse rejection. It now says such a run arrives with no
    error at all, and is identified by status and the absent speedup. Your
    point stands: SystemExit is not caught by except Exception, so neither
    side of the truncation ever sees those rows.
  • The compactness claim is gone. The row does gain status and elapsed_s,
    as test_gemm_trace_clean_run_row_stays_compact asserts.

@iraj465

iraj465 commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

A/B on the claw CI corpus: what this PR changes about GEMM-tuning visibility

Scope, stated up front. This PR does not move Hyperloom e2e, and no table
below claims that it does. It is an audit-trail fix. Its A/B is what the trace row
can express about a tuning run, measured against ground truth in tune.log.

Backend disclosure. Of the 388 gemm_tuning trace rows on record, 384 are
forge and 4 are geak
. This is predominantly the Forge path, not GEAK.

Replay limitation. A true two-arm replay is not possible here. The handler's
input — the tuner result payload — is not persisted; only the row it produced
is. So the control column is the row as actually written across the campaign,
and the test column is what this PR's builder emits from the same fields.

Corpus, measured 18 Aug 2026 over /shared_nfs/hyperloom-claw/: 322
reports/trace/gemm_tuning.jsonl files, 388 rows, 366 per-tuner entries,
375 tune.log files.

1. What the audit row holds

control (on record) test (this PR)
keys per tuner entry exactly 3 — tuner, best_micro_speedup, kept (all 366 entries) those 3, plus status, elapsed_s, error_class, error when present
kept non-null 0 of 366 0 of 366 — unchanged, see below
ok rows carrying error_class 0 of 320 0 of 320 — invariant preserved
a tuner that crashed vs one that found nothing identical rows distinguishable by status / error_class

2. The population that is currently unreadable

135 of 366 tuner entries carry no speedup. Today a crash and an honest "found
nothing" produce the same row, so the audit cannot tell them apart:

where those 135 entries sit n
in a run whose status is ok 122
recorded micro_decision: no_improvement — a clean tuning verdict 107
recorded micro_decision: failed 13
recorded micro_decision: empty_output 6

By tuner: sglang_dense_bf16 101, fmoe_ck 27, a8w8 4, a8w8_blockscale 2,
a4w4_blockscale 1.

3. Ground truth: how many of those "no improvement" verdicts are real

From the tune.log files, independent of the trace:

n
tune.log files 375
logs whose tuner rejected its own arguments (unrecognized arguments: --libtype) 93
...all in sglang_dense_bf16 (of its 99 logs) 93
...whose wrapper header says # Exit code: 0 while the inner command exited with code 2 93 of 93

So 93 runs died before tuning anything, every one of them reported success, and
the audit row recorded a clean verdict. That is the failure this PR makes
greppable — the tuner's own status and error_class survive into the row
instead of being flattened to a missing speedup.

4. What this PR does NOT fix

kept is still null in all 366 entries Nothing on the GEMM path writes it. The write-back that would, _validate_forge_gemm_tuning_e2e, emits a separate e2e_results list and runs after this trace fires. That gap is real and is left alone here rather than papered over. kept is kept in the row for shape stability only.
the --libtype rejection itself The flag is built inside forge_gemm_tune (KernelForge), a third repo vendored in neither Hyperloom nor GEAK. Tracked as #1211.
e2e Nothing here changes a measured throughput.

5. Review items, closed

  • error_class promotion is now gated on
    envelope_failed = str(result.get("status") or "").strip().lower() == "failed",
    so a tuner failing inside a successful run never stamps the row. Verified: 0 of
    320 ok rows carry an error_class, while 121 of them hold a tuner that
    returned no speedup and would have been promoted ungated.
  • The kept claim is corrected in the comment rather than asserted — see section 4.
  • The _TRACE_ERROR_CHARS rationale now states that an argparse rejection reaches
    neither side of it: the downstream tuner's rejection is a returncode from
    subprocess.run, and the wrapper's own is a SystemExit, which its
    except Exception does not catch. Such a run arrives with no error field
    and is identified by status and the absent speedup instead.

Counts in the source comments are re-measured and date-stamped as of 18 Aug 2026
(280e222a2); the campaign is live, so they drift upward. 337 unit tests pass.

iraj465 and others added 4 commits August 18, 2026 17:17
``_trace_gemm_tuning_run`` copied only ``tuner`` / ``best_micro_speedup`` /
``kept`` out of each ``tuners_run`` entry. A tuner that crashed and a tuner
that ran to completion and found nothing both arrive with no speedup, so the
audit trail could not tell them apart, and the run envelope leaves
``error_class`` unset even when every tuner failed.

Keep the tuner's own ``status``, ``elapsed_s``, ``error_class`` and the head of
its ``error`` (400 chars), and promote the first tuner's ``error_class`` to the
envelope when the envelope has none. The three legacy keys stay unconditional;
the new ones are omitted when empty, so a clean row is unchanged.

Campaign evidence (/shared_nfs/hyperloom-claw, 300 gemm_tuning.jsonl files,
361 rows): 23 tuner runs exited non-zero and every one is recorded today as an
ordinary result. Replaying the real result.json through the patched function:

  Qwen3-8B/20260813T091428Z          a8w8       failed   -9  500.4s subprocess_error
  DeepSeek-V4-Pro/20260815T002915Z   fmoe_ck    failed    1  288.5s subprocess_error
  Llama-3.1-8B-Instruct/20260813T091520Z sglang_dense_bf16 no_improvement 11.2s
  Qwen3-8B/20260730T122359Z          sglang_dense_bf16 ok 15.63x       1584.9s

The last two rows are the point: both are ``sglang_dense_bf16``, both were
audited identically before this change, and 11.2s against 1584.9s is the only
thing that separates a tuner that died in argparse from one that tuned 28 of
28 shapes.
Three review items on the GEMM-tuning audit row.

1. The promotion was ungated. A run can succeed on one tuner while another
   fails, and `result.get("error_class") or tuner_error_class` stamped that
   run's row with the failing tuner's class -- reporting the whole run as
   failed by it. Gate on the envelope's own verdict.

   Measured over the 320 `gemm_tuning.jsonl` traces in the claw campaign
   (386 rows, 364 tuner entries):

     ok rows carrying an error_class today            0 of 318
     ok rows with >=1 tuner that produced no speedup      119
     failed rows the promotion makes greppable            <=12

   `error_class implies the run did not succeed` holds perfectly in the
   corpus today. Ungated, the promotion risks that invariant on up to 119
   rows to reach at most 12. Every one of the 12 is `status: failed`, so
   gating on `failed` keeps the entire benefit and gives up none of it.
   (119 is an upper bound on exposure: the pre-patch trace drops per-tuner
   error_class, so which of those tuners carried one cannot be read back.)

2. The `_TRACE_ERROR_CHARS` comment offered "an argparse rejection" as the
   failure it identifies. That is the one failure it cannot see. A rejected
   argument reaches neither side of the wrapper: the downstream tuner's
   rejection is a returncode out of `subprocess.run`, and the wrapper's own
   is a `SystemExit`, which its `except Exception` does not catch. Either
   way the run arrives with no `error` field at all. Replaced with failures
   that do produce one, and said what identifies the other kind instead.

3. `_TRACE_TUNER_KEEP_NULL` claimed "existing readers index on them" for all
   three keys, and that a clean run's row "stays as compact as it was".
   Neither is right for `kept`: nothing on the GEMM path writes it, and it
   is null in all 364 tuner entries on record, so a reader can only be
   indexing on its presence. And the row is not as compact as it was --
   `status` and `elapsed_s` are new and non-null on clean runs, which is
   what `test_gemm_trace_clean_run_row_stays_compact` itself asserts.
   `kept` stays emitted, for shape only; both claims now say what is true.

Sixth test asserts a `status: ok` run with one failed tuner keeps a clean
row while the tuner's own error_class survives on its entry. 337 tests in
the file pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment quoted 364 tuner entries across 320 traces. The campaign is
live, so re-measured 18 Aug 2026: 366 entries across 322 traces, 0 of
them non-null. The claim holds; the numbers are now current.

Also names the producer that would populate the field, per review:
`_validate_forge_gemm_tuning_e2e` writes a separate `e2e_results` list
and runs after this trace fires. Saying "nothing writes it" without
saying what almost does left the reader to re-derive it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The campaign is live, so the census drifted: 320 ok rows, not 318, and 121
of them carry a tuner with no speedup, not 119. The invariant the comment
justifies is unchanged -- 0 ok rows carry an error_class -- so only the
sample size is restated, now date-stamped so the next reader knows when.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@iraj465
iraj465 force-pushed the fix/gemm-tuning-trace-keeps-failures branch from 280e222 to 363a8ba Compare August 18, 2026 17:53
@iraj465
iraj465 marked this pull request as draft August 19, 2026 03:32
@iraj465

iraj465 commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator Author

Parked as draft

In last few Hyperloom CI runs, there is little evidence that GEAK gemm tuning
integrate result into Hyperloom does have downstream issues, so parking it for
now to reduce last minute regressions due to low testing.

Supporting split, re-measured over /shared_nfs/hyperloom-claw/:

trace rows per-tuner entries
backend: forge 397 378
backend: geak 4 0

All 4 GEAK rows are status: skipped, error_class: legacy_geak_config_missing,
tuners_run: []. They never reach a tuner, so the per-tuner failure signal this
PR preserves does not appear on the GEAK side anywhere in this corpus.

_trace_gemm_tuning_run is shared code — run_gemm_tuning_handler calls it on
both branches — so this is a statement about observed traffic, not about the call
graph.

Related issues #1210 and #1211 stay open.

Nothing here is retracted. The 337 unit tests pass and the review items in the
A/B comment above are closed. Mark ready for review when this comes back up the
priority list.

Corpus counts date-stamped 19 Aug 2026; the campaign is live and they drift
upward (they were 388 / 366 on 18 Aug).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants