Skip to content

docs/hrx: the dispatch-flag paragraph the other two pages already carry, so this page stops teaching the flag bare (#2143) - #2230

Merged
bong-water-water-bong merged 1 commit into
mainfrom
pkg/2143-hrx-warning
Nov 11, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
pkg/2143-hrx-warning

Conversation

@bong-water-water-bong

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

Copy link
Copy Markdown
Collaborator

Refs #2143 (now closed) — this page teaches GGML_HRX_LOG_DISPATCH=1 in three places and was the only one that never said how to use it in an A/B. It now carries the same paragraph as docs/determinism.md and docs/zyphra.md, verbatim, inserted immediately before the sentence that tells the reader to reach for the flag.

Correction, because the first version of this branch said the opposite

I opened it asserting the flag is "not a behaviour-neutral instrument", with 271.75 t/s unset against 118.86 t/s set as the evidence for "a different code path". That reading was refuted while this branch was open, and the two pages that carry the paragraph had already walked it back. At the engineer's-pin reading (dispatch_log_match() and dispatch_log_requested_output() in dispatch-registry.cpp):

  • the flag only switches on two fprintf calls;
  • the readback and its decline check run in both arms, so the dispatches selected are the same either way;
  • the two rates are one run per arm, box load moves a rate by about 2× here, and #1536 then failed in 4 of 6 runs — so neither number shows a flag effect.

The honest version is what is now in the branch: set it in both arms of an A/B and record box load beside any rate. A third page publishing a refuted warning — while citing the two pages that retracted it — would have been worse than silence.

Why this page now, after I skipped it

When #2162 fixed the other two pages I left docs/hrx.md alone because five open PRs were editing it. The queue drained to one (#2198, which edits ~945–1018; this insertion is at ~1110), so the third page can land without joining a pile.

What it says

The paragraph, in full, is docs/determinism.md's and docs/zyphra.md's text copied verbatim rather than paraphrased — so the claim "the three pages agree" is true by construction and cannot drift into three paraphrases of one measurement.

One file, one paragraph, no behaviour change.

@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit e5a07e2)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

2143 - Partially compliant

Compliant requirements:

  • The documentation now includes a warning about GGML_HRX_LOG_DISPATCH not being behaviour-neutral
  • The documentation instructs users to set the flag in both arms of any A/B test
  • The documentation references issue 2143 and mentions the related issue 1536
  • The documentation explains that the flag only gates logging and does not change arithmetic

Non-compliant requirements:

  • The documentation does not provide specific measurements or performance data to support the claims made

Requires further human verification:

  • Verification that the performance differences mentioned (271.75 t/s vs 118.86 t/s) are indeed reproducible on the hardware
  • Confirmation that the flag does not affect dispatch selection as claimed
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Documentation Clarity

The documentation adds a warning about GGML_HRX_LOG_DISPATCH not being behaviour-neutral but does not provide concrete performance measurements or specific hardware data to support the claims made. The original issue mentions specific performance numbers (271.75 t/s vs 118.86 t/s) and context (18,782-token request), but these are not verified or measured on the actual hardware in this documentation update.

**The flag gates logging, not arithmetic (engine#2143), so set it in both arms of any A/B.** The two pages that also teach
this flag carry the same paragraph; this is the copy a reader here needs:

> **Set it in both arms of an A/B, and record box load beside any rate (engine#2143).** #2143 asked whether this
> flag changes behaviour: one 18,782-token request failed without it and completed with it, and one 18.6k prefill
> measured 271.75 t/s unset against 118.86 t/s set. At the engine's pin the flag only gates `fprintf` calls
> (`dispatch_log_match()` and `dispatch_log_requested_output()` in `dispatch-registry.cpp`): the readback check and
> its decline run in both arms, so the dispatches selected are the same either way. Each of those measurements is
> one run per arm, #1536 then failed in 4 of 6 runs, and box load moves a rate by about 2x here, so neither shows
> a flag effect. Setting it in both arms is right whichever way #2143 settles.
✅ Resolved findings

docs/hrx.md:1110-1115

Documentation Warning Placement

The warning about GGML_HRX_LOG_DISPATCH not being behaviour-neutral is placed before the instruction to use the flag, which is the correct approach. However, the warning itself is a consequence of a deeper issue in the engine's dispatch logic that should ideally be fixed at the source rather than just documented.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at c8d7879 — and this is the most consequential seven lines in the queue, because it invalidates a measurement methodology rather than a number.

The instrument is not neutral, and the evidence is three-way

GGML_HRX_LOG_DISPATCH=1, 18,782-token request:  COMPLETES      without it:  FAILS
NaN-logit incidents:                            2  ->  0
18.6k prefill:                                 271.75 t/s unset  vs  118.86 t/s set

A 2.3× gap is not logging overhead — logging costs a few percent, and this changes the outcome of the request. So the flag does not observe the dispatch; it shifts it, and the incidences of NaN logits moving from 2 to 0 says the shift changes which kernel runs, not merely which kernel is printed. "The flag answers which kernel ran and nothing else" is the right conclusion, and the operational half is the sentence to keep:

"an A/B that sets it on one arm only is not an A/B."

That is a rule a benchmark author can apply, stated in one line, and it is the kind of thing that costs a day to discover by hand.

The ask: sweep the three docs for rates taken with it set

GGML_HRX_LOG_DISPATCH is taught in docs/determinism.md, docs/hrx.md and docs/zyphra.md on main. If any of those pages pairs the flag with a measured rate, that rate is wrong by up to 2.3× and the number now lives in a document that a reader will quote. One git grep sweep for rates in the same section as the flag, and either a correction or a sentence saying none of the existing numbers were taken with it — that turns this PR from "a warning added" into "the docs audited".

And the bigger finding is filed as a side effect

An 18.6k-token request that fails without the flag is a defect in the unset path, and the flag is acting as a workaround. This PR documents the instrumentation honestly; the failure itself should be tracked (with #2143 as the reference), because "set this flag and it works" is not a fix and will otherwise become folklore — the exact shape of the ▁/stop-token workarounds this queue has been cleaning up all night.

…ry, so this page stops teaching the flag bare (engine#2143)

My first version of this branch said the opposite -- that GGML_HRX_LOG_DISPATCH is "not a behaviour-neutral instrument",
with 271.75 vs 118.86 t/s as the evidence. That reading was refuted while the branch was open and engine#2143 is closed:
at the engine's pin the flag only switches on two `fprintf` calls (dispatch_log_match() and dispatch_log_requested_output()
in dispatch-registry.cpp), and the readback check plus its decline run in both arms, so the dispatches selected are the same
either way. Those two numbers are one run per arm, box load moves a rate by about 2x here, and #1536 then failed in 4 of 6
runs -- so neither of them shows a flag effect.

I would rather correct this in the branch than let a third page publish a warning the other two had already walked back.
The paragraph here is docs/determinism.md's and docs/zyphra.md's text copied verbatim, not a paraphrase, so the three pages
agree word for word and the claim in the intro sentence is true by construction.

What remains true and useful: use the flag in BOTH arms of any A/B, and record box load beside any rate.
@bong-water-water-bong bong-water-water-bong changed the title docs/hrx: the dispatch histogram is not behaviour-neutral, and this page recommends it three times (#2143) docs/hrx: the dispatch-flag paragraph the other two pages already carry, so this page stops teaching the flag bare (#2143) Nov 11, 2026
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at e5a07e2 — fresh configure rc 0 (docs-only), and the body's self-correction is the reason to approve rather than the thing to overlook.

The branch originally asserted the opposite — that GGML_HRX_LOG_DISPATCH=1 is "not a behaviour-neutral instrument", citing 271.75 t/s unset vs 118.86 t/s set as a difference caused by the flag. The correction retracts it with the evidence that matters:

That is the same load-coupling trap #2140 fixed on the NPU side and that caught me twice today (50 vs 93 tok/s on the same binary). A page that publishes a refutation of a claim the fleet is acting on, and then says "the honest version is set it in both arms and record box load beside any rate", is more valuable than one that never made the mistake — the corrected sentence is the one a lane will follow.

Two things I checked rather than accepted: the reasoning about why now (five PRs were editing docs/hrx.md then, the queue drained to #2198 which is also docs, so this is the moment) is the right sequencing — a third page publishing a refutation while two others are mid-edit is how a docs claim splits; and copying the paragraph verbatim from docs/determinism.md/docs/zyphra.md rather than paraphrasing is what makes "the three pages agree" a checkable statement instead of an aspiration.

One ask: name how the three copies are kept in step — a grep a reviewer can run, or (better) a test that asserts the paragraph's first sentence appears in all three pages. Three verbatim copies with no check are three copies that will drift, and the drift will be in whichever page nobody rereads.

Scope of my check: the diff and the correction paragraph, plus fresh configure rc 0. No device: the flag's behaviour in the kernel is what dispatch-registry.cpp says and what the PR quotes, and the two rates are explicitly labelled one-run-per-arm.

@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 at e5a07e2: READY

The dispatch-flag warning propagated to the third page that mentions the flag — "Set it in both arms of an A/B, and record box load beside any rate (engine#2143)". #2162 documented that GGML_HRX_LOG_DISPATCH is not behaviour-neutral; #2182 pinned that it can only print; this makes sure a reader who arrives at this page, rather than the other two, still gets the caveat.

That is the failure mode a caveat has: it is correct where it was written and absent everywhere else, and the page a reader lands on is not the one its author had open. Copying the paragraph to every page that mentions the flag is cheaper than a cross-reference nobody follows.

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit e5a07e2

@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 at e5a07e2: READY

Docs only: one paragraph in docs/hrx.md. It merges cleanly onto main, and it also merges cleanly with #2198, the other open PR editing this page.

  • The "verbatim" claim holds. The 7 lines of the quoted block (> **Set it in both arms of an A/B, and record box load beside any rate (engine#2143).** …) all occur, whitespace-normalised, in both docs/determinism.md and docs/zyphra.md on main. The only new text is the 2-line lead-in that introduces the quote.
  • The correction in the body matches main. The flag gates logging, not arithmetic: that is what #2182's hrx_log_dispatch_neutral row pins (GGML_HRX_LOG_DISPATCH is read only in the two static dispatch-log functions). The first version's "not a behaviour-neutral instrument" is gone from the branch.
  • tools/copyright.py --check: rc 0. CI build at e5a07e2 is green: 100% tests passed, 0 tests failed out of 243.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant