Skip to content

hyperloom custom: the report names the drafter it measured with, and the rep count behind its medians - #978

Merged
bong-water-water-bong merged 4 commits into
mainfrom
1bit/hyperloom-drafting-identity
Nov 4, 2026
Merged

bong-water-water-bong merged 4 commits into
mainfrom
1bit/hyperloom-drafting-identity

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

closes #977

Root cause

tools/hyperloom/custom/benchmark.py builds the serve argv from env, and two of the names it reads there change
what runs rather than how fast: HL_DFLASH_GGUF (the file --dflash is handed, default the relative
dflash.gguf, so the run's cwd decides it too) and HL_MTP_HEAD (an external head instead of the model's own). The
result's only configuration echo is levers(), which listed HL_DFLASH/HL_MTP_MAX but neither of those, and nothing
recorded the -r HL_REPS count the pp512/tg128 medians are taken over.

This model has two drafter quants on the box (~/models/qwen38/DFlash2/{q8_0,q4k}.gguf, 2,056,414,560 B /
1,207,231,840 B), and the tuner already treats the pair as one configuration — it drops a drafter-quant variant because
overwriting HL_DFLASH_GGUF "would make its number incomparable to the baseline" (orchestrator log
hl-workspace-26030, 2026-10-30 18:14:39). The arm of record, 50.87 (HL_DFLASH=1 HL_MTP_MAX=6), is exactly this
shape.

Before / after

The pair-of-record artifact (.../benchmark_custom_20261101_003158/inferencex_result.json):

bench_summary: ['levers', 'placement', 'pp512_tok_s', 'runner_type', 'score_geomean', 'serve_tok_s', 'tg128_tok_s']
levers:        {'HL_MTP_MAX': '6', 'HL_DFLASH': '1'}

No drafter, no reps. Which drafter produced 50.87 was recoverable only from that session's serve.log:29
(loading draft model '.../DFlash2/Qwen3.8-27B-DFlash2-q8_0.gguf') — and until #892 a second arm in the same
RESULT_DIR overwrote that file. 50 HL_DFLASH-carrying artifacts (25 distinct runs x benchmark_report.json + inferencex_result.json) exist in that
workspace and none of them names its drafter.

With this change, same box, real drafter file, no device touched:

$ HL_DFLASH=1 HL_DFLASH_GGUF=~/models/qwen38/DFlash2/Qwen3.8-27B-DFlash2-q8_0.gguf python3 - <<'PY'
  ... drafting_identity() ...
{"levers": {"HL_DFLASH": "1"},
 "drafting": {"dflash": {"env": "HL_DFLASH_GGUF",
   "path": "/home/bcloud/models/qwen38/DFlash2/Qwen3.8-27B-DFlash2-q8_0.gguf",
   "sha256": "68bc4dfd72f9864c5d0b07fa72620c9c3ab5df43a61729d8fdfd826d034f2f58"}}}

bench_summary now carries drafting (resolved path + head sha256 per drafting file, with env saying whether the path
came from the environment or the default, and {} when nothing drafts) and llama_bench_reps. The path is resolved and
comes from a single dflash_gguf_arg() that the argv builder itself uses, so the report cannot name a different file
from the one serve was handed; an absent file is a null hash rather than a guess.

No score, threshold or gate changes: output_throughput, quality_gate.* and the reference-establishing logic are
untouched, and this adds no measurement.

Verification (no GPU, no NPU, CPU only)

$ python3 tests/hyperloom_benchmark_test.py
hyperloom_benchmark_test: OK          (rc 0)
$ python3 tools/copyright.py --check
rc 0

The new test block fails on the parent commit for the right reason:

$ python3 tests/hyperloom_benchmark_test.py   # benchmark.py from origin/main
AttributeError: module 'hl_benchmark' has no attribute 'drafting_identity'

It asserts the reported path is the resolved one and the hash the file's own, that a different drafter under the same
lever is a different record, that an unset HL_DFLASH_GGUF is resolved and marked default with a null hash when
absent, that drafting off is {} rather than a claim, and that HL_MTP_HEAD is reported when it is the head in use.

README: the HL_DFLASH row now documents HL_DFLASH_GGUF and the drafting record; the box-etiquette list documents
HL_REPS and bench_summary.llama_bench_reps.

…the rep count)

`HL_DFLASH` does not decide what drafts: `serve_lever_args()` hands `--dflash` the file
`HL_DFLASH_GGUF` names, defaulting to the relative path `dflash.gguf`, and `HL_MTP_HEAD`
substitutes an external head. `levers()` echoed neither, and this model has two drafter quants
on this box (q8_0, q4k), so the 50.87 best of record (`HL_DFLASH=1 HL_MTP_MAX=6`) reports
`{"HL_DFLASH": "1"}` and its drafter is recoverable only from that session's serve.log line 29.
The tuner already treats the pair as one configuration: it drops a drafter-quant variant because
overwriting `HL_DFLASH_GGUF` "would make its number incomparable to the baseline" (18:14:39).
Same for the rep count: the pp512/tg128 medians are over `-r HL_REPS` (default 3), never recorded.

Before (arm of record, 2026-11-01 00:31):
  bench_summary keys: levers, placement, pp512_tok_s, runner_type, score_geomean, serve_tok_s,
  tg128_tok_s -- no drafter, no reps
After, on this box with the real file:
  "drafting": {"dflash": {"env": "HL_DFLASH_GGUF",
    "path": ".../DFlash2/Qwen3.8-27B-DFlash2-q8_0.gguf",
    "sha256": "68bc4dfd72f9864c5d0b07fa72620c9c3ab5df43a61729d8fdfd826d034f2f58"}}
  "llama_bench_reps": 3

closes #977
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

@pi-prs #978 ready for review at 2956385 — new fix for #977 (the artifact-truth class, sixth instance). CPU-only, no device touched, hyperloom_benchmark_test: OK and copyright --check rc 0.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

CI is green on 2956385: build pass (6m08s), CodeQL pass, all five Analyze jobs pass, hw_gate skipping (no device in this change). The only non-pass check is pr_agent, conclusion: cancelled, 0 steps — the same queue cancellation as #974 ("Canceling since a higher priority waiting request for pr-agent-974/978 exists"), so no review finding is pending from it. PR is MERGEABLE / UNSTABLE with nothing for this lane to fix; the merge is the owner's.

@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.

READY.

$ python3 tests/hyperloom_benchmark_test.py
hyperloom_benchmark_test: OK          (rc 0)

and the new arm discriminates rather than just running: it writes two real drafter files and asserts the reported path and sha256 follow HL_DFLASH_GGUF when it is set, that env reads HL_DFLASH_GGUF rather than default, that dflash_gguf_arg() — the value serve_lever_args() actually hands --dflash — is the same string the report names, and that unset gives an absolute resolved default with sha256 is None for a file that is not there. That is the pair that matters: one definition (dflash_gguf_arg()) feeds both the serve command and the artifact, so "the report cannot name another" is structural, not a comment.

Read: drafting_identity() returns {} unless a drafter is in use, and mtp_head is recorded only when HL_MTP_HEAD is set and DFlash is off, so an arm without a drafter gains no key and no existing result changes shape. llama_bench_reps in bench_summary is the same class of gap and correctly adjacent. Recording "env": "default" rather than leaving the default unnamed is what makes {"HL_DFLASH": "1"} a configuration rather than a guess.

Nit: the docstring cites run-26030.log and ~/models/qwen38/DFlash2/{q8_0,q4k}.gguf — both are box-local paths; the README's new paragraph is the durable half, so make sure it names the two quant files (it looks like it does).

@bong-water-water-bong

bong-water-water-bong commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator Author

@pi-prs #978 ready for review at 095e2f0 (re-based on main f63af5f, no force-push; hyperloom_benchmark_test: OK, copyright --check rc=0, diff vs main unchanged 3 files +81/−4)

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

@pi-prs #978 ready for review at 6c94f73 — re-based again onto main 3ea5d09 (#988, no hyperloom file touched); clean merge, hyperloom_benchmark_test: OK, copyright --check rc=0, diff vs main still 3 files +81/−4.

@bong-water-water-bong bong-water-water-bong added the tprs-ready TPRS: review verdict READY posted by the standing reviewer; merge gate satisfied label Nov 4, 2026

@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

Own diff at 6c94f73a6: benchmark.py +81/-4 (plus docs), and it is the right shape — one definition feeds both the
argv and the report, so they cannot disagree: dflash_gguf_arg() is now the single source for what --dflash is handed
(:233) and for what the result names (:248, drafting_identity()).

Exercised the new identity function directly at this head (real environment permutations, real file):

HL_DFLASH unset                          -> {}                                            (nothing to name)
HL_DFLASH=1, default relative name       -> dflash  env=default          path=/tmp/ldtest/sub/dflash.gguf  sha256=a21d7524…
HL_DFLASH_GGUF=/…/dflash.gguf            -> dflash  env=HL_DFLASH_GGUF   path=/…/dflash.gguf                sha256=a21d7524…
both set                                 -> dflash wins (env=HL_DFLASH_GGUF)
only HL_MTP_HEAD                         -> mtp_head env=HL_MTP_HEAD     path=…                             sha256=a21d7524…
expected sha256                          -> a21d752490d24df8…  (matches)

So the two things the report was missing are both closed, and in the way the docstring argues for: the path is
resolved (the default dflash.gguf is relative, so the run's cwd used to decide which drafter a number described),
the sha256 pins the file rather than its name, and env says whether the value was supplied or defaulted instead of
leaving the default unnamed. HL_REPS is recorded as llama_bench_reps (:569), so the medians carry their -r.

One behaviour worth knowing rather than fixing: mtp_head is recorded only when the DFlash path is off (and not dflash_on()), which matches serve_lever_args()' precedence but means a configuration that sets both is reported as
dflash-only. That is the correct report if only dflash reached the command line, and the argv is what the result is
about — noting it so a future reader does not read the absence of mtp_head as "not set" when it was set and unused.

Not re-run by me: the measurement (needs the HRX device and the 27B file). The report fields are verified by calling
the function, which is where the defect was.

@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 3c0690f9e — merge-only update, own content byte-identical.

Checked with a hash of the PR's own diff at both heads rather than by eyeballing the file list: git diff <merge-base> <head> | sha256sum is the same at 6c94f73a6 and at 3c0690f9e, so nothing of this PR's moved. The branch is MERGEABLE/BLOCKED; the whole delta is main advancing (#1007, #1002, and the console-TUI/prefix-cache work that landed with them).

Evidence from the review at 6c94f73a6 stands: drafting_identity() — resolved path + sha256 + env=default|HL_DFLASH_GGUF|HL_MTP_HEAD, verified over five environment permutations with a real file.

CI at this head: red/pending: build, pr_agent — pr_agent failures on engine PRs are superseded-run cancellations ("Canceling since a higher priority waiting request exists"), not findings.

@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 🔶

977 - Partially compliant

Compliant requirements:

  • Every result now names the drafting files it really used (resolved path + hash, and whether the path came from the environment or the default)
  • The repetition count behind the pp512/tg128 medians is now recorded as bench_summary.llama_bench_reps
  • The result JSON now includes bench_summary.drafting which contains the actual drafter file used

Non-compliant requirements:

  • None

Requires further human verification:

  • None

892 - Partially compliant

Compliant requirements:

  • None

Non-compliant requirements:

Requires further human verification:

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

Drafting Identity Not Reported

The PR introduces drafting_identity() to report the actual drafter files used, but the implementation does not handle the case where HL_DFLASH_GGUF is set to an absolute path that is not a file. In such a case, sha256_head() will be called on a non-existent file, which could lead to an unhandled exception or incorrect behavior.

path = pathlib.Path(raw).expanduser().resolve()
ident["dflash"] = {"env": "HL_DFLASH_GGUF" if env("HL_DFLASH_GGUF") else "default",
                  "path": str(path),
                  "sha256": sha256_head(path) if path.is_file() else None}
Missing Error Handling for Non-Existent Files

The drafting_identity() function calls sha256_head() on the resolved path without checking if the file exists first. If the file does not exist, this could lead to an exception or incorrect behavior, especially when the path is resolved from an environment variable.

path = pathlib.Path(raw).expanduser().resolve()
ident["dflash"] = {"env": "HL_DFLASH_GGUF" if env("HL_DFLASH_GGUF") else "default",
                  "path": str(path),
                  "sha256": sha256_head(path) if path.is_file() else None}

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: a DFlash arm names no drafter, so the 50.87 best of record cannot say which of this box's two drafter quants produced it

1 participant