Skip to content

hyperloom custom workload: record the device stack (kernel + HSA runtime) with each measurement - #948

Merged
bong-water-water-bong merged 1 commit into
mainfrom
1bit/hyperloom-device-stack
Nov 4, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
1bit/hyperloom-device-stack

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Closes #947.

Root cause

bench_summary.build names the binary (#933) but nothing names the device stack, so the pair-of-record artifacts of the long-context leg are tied to their kernel by prose only. The box rebooted five times in 18 h on 2026-11-03/04 and the tuner launcher applies no kernel gate (only the device-free window harness does), so a hyperloom optimize run on an unvalidated boot would write numbers into a report that cannot say which stack produced them. placement is not a substitute: it is null in those same JSONs.

Before / after (same file, bench_summary keys)

before: levers, longctx, placement, pp512_tok_s, runner_type, score_geomean, serve_tok_s, tg128_tok_s
after:  ... + device_stack
        {"kernel": "7.3.0-rc4-1bit-perfopt+",
         "hsa_runtime": "/opt/rocm-therock/lib/python3.14/site-packages/_rocm_sdk_core/lib/libhsa-runtime64.so.1",
         "hsa_runtime_sha256": "21a6682452e159cfcdc6bdc84676f6b7f9c1236f3a2957a97a2e7170889cdfa3"}

hsa_runtime is the file IREE_HAL_AMDGPU_LIBHSA_PATH names (falling back to the box's ROCm-TheRock path when it exists) and is null when there is none — the path is reported, so nothing is attributed to a runtime it did not load. sha256_head is the helper build_identity already uses.

Verification

  • python3 tests/hyperloom_benchmark_test.py → hyperloom_benchmark_test: OK (new asserts: kernel == platform.release(), a stub runtime hashes to hashlib.sha256 of its bytes, a missing path yields null).
  • python3 tools/copyright.py --check → rc=0.
  • Field printed live on this box (the JSON above).

No measurement is affected: nothing on the scoring path reads this, and the value is written once per benchmark invocation.

Two files, +41/−1. Not merged — the owner merges in the morning.

… each measurement

The report named the binary (#933) but not the device stack it ran on. This box
rebooted five times in 18 h on 2026-11-03/04 and the tuner launcher applies no
kernel gate, so a number could only be tied to a kernel by the prose in
~/briefs/THYPERLOOM-STATE.md: the pair-of-record JSONs carry
bench_summary{levers,longctx,placement,pp512_tok_s,runner_type,score_geomean,serve_tok_s,tg128_tok_s}
and nothing about the kernel or the HSA runtime.

bench_summary.device_stack = {kernel, hsa_runtime, hsa_runtime_sha256}, the
runtime being the file IREE_HAL_AMDGPU_LIBHSA_PATH names (whole-file hash, same
helper as the build field) or null when there is none -- named by path, so no
number is attributed to a runtime it did not load.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

#948 ready for review at 358ad99 — new field bench_summary.device_stack {kernel, hsa_runtime, hsa_runtime_sha256}; hyperloom_benchmark_test: OK rc=0, tools/copyright.py --check rc=0, two files +41/−1, no change on the scoring path. Closes #947.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

947 - Partially compliant

Compliant requirements:

  • Added device_stack field to bench_summary with kernel, hsa_runtime, and hsa_runtime_sha256
  • Kernel version obtained via platform.release()
  • HSA runtime path read from IREE_HAL_AMDGPU_LIBHSA_PATH with fallback to ROCm-TheRock path
  • SHA256 hash computed using sha256_head helper
  • When HSA runtime file does not exist, hsa_runtime is set to null

Non-compliant requirements:

  • None

Requires further human verification:

  • None

933 - Partially compliant

Compliant requirements:

  • Added build field to bench_summary with onebit path, bin_dir, and sha256
  • SHA256 hash computed using sha256_head helper

Non-compliant requirements:

  • None

Requires further human verification:

  • None
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing Error Handling for Non-Existent HSA Runtime File

The device_stack() function attempts to access the HSA runtime file specified by IREE_HAL_AMDGPU_LIBHSA_PATH or the default ROCm-TheRock path, but does not handle the case where the file exists but cannot be read due to permissions or other filesystem issues. This could lead to an unhandled exception during runtime, potentially crashing the benchmark process.

libhsa = pathlib.Path(env("IREE_HAL_AMDGPU_LIBHSA_PATH") or
                      "/opt/rocm-therock/lib/python3.14/site-packages/_rocm_sdk_core/lib/"
                      "libhsa-runtime64.so.1").expanduser()
return {
    "kernel": platform.release(),
    "hsa_runtime": str(libhsa) if libhsa.is_file() else None,
    "hsa_runtime_sha256": sha256_head(libhsa) if libhsa.is_file() else None,
}

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 358ad99 (tprs-ready added). build SUCCESS; pr_agent CANCELLED (the expected state with coder-llm off); everything else green.

Run, not read: python3 tests/hyperloom_benchmark_test.py at this head → hyperloom_benchmark_test: OK, rc 0. And device_stack() against this box, loaded the way the report loads it:

{'kernel': '7.3.0-rc4-1bit-perfopt+',
 'hsa_runtime': '/opt/rocm-therock/lib/python3.14/site-packages/_rocm_sdk_core/lib/libhsa-runtime64.so.1',
 'hsa_runtime_sha256': '21a6682452e159cfcdc6bdc84676f6b7f9c1236f3a2957a97a2e7170889cdfa3'}

That hash is the runtime the c22 package's own 1bit serve prints for the same file (I checked that separately on #946 today), so the field names the right object. The three branches in the test (env set to a real file, to an absent path → None, and the default TheRock path) all behave as the docstring says.

Nothing downstream rejects the new key: Hyperloom's bypass_report.py:90-99 copies bench_summary into the report wholesale whenever it is not None, so an extra sub-dict is carried, not validated. Verified by reading the executor, not by assumption.

Two nits, neither gating:

  1. The docstring says "the runtime is named by path AND whole-file hash", but it uses sha256_head(), which hashes the first 64 MiB. For libhsa-runtime64.so.1 (5.1 MB) that is the whole file, so the value is right; the sentence is what would mislead a later reader if the same helper were pointed at something bigger. "…and its sha256 (a head hash: 64 MiB cap, the whole file for a 5 MB runtime)".
  2. The last assert calls bench.device_stack() twice (...["hsa_runtime"] is None, bench.device_stack()) — the message re-evaluates it; harmless, just noting it.

Also for merge order: #950 adds its asserts to the same main() region of tests/hyperloom_benchmark_test.py that this PR touches, so the two will conflict textually; whichever lands second needs a trivial rebase.

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

Labels

Review effort 3/5 tprs-ready TPRS: review verdict READY posted by the standing reviewer; merge gate satisfied

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hyperloom custom workload: the result JSON names the binary but not the kernel or HSA runtime, so a number cannot be tied to a device stack

1 participant