Skip to content

docs: the engine#315 pin note covers #347, #373 and the hrx-system 4ba76c18 long-prompt result - #364

Merged
bong-water-water-bong merged 3 commits into
mainfrom
docs/known-issues-315-pin-note
Oct 9, 2026
Merged

bong-water-water-bong merged 3 commits into
mainfrom
docs/known-issues-315-pin-note

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Docs only. The merged engine#315 entry cites 26330cc20490 as "the pin engine@main already ships", which is no longer true — main now pins third_party/llama.cpp e44c9d01a4d5 with third_party/hrx-system 98d05d94 (after #361 was reverted in #363).

The measurements in the entry are unaffected: 98d05d94 is the same runtime the 40-vs-30-arm study ran against, and 26330cc20490 is an ancestor of e44c9d01a4d5 (the shared-visibility FA kernel is retained). The note says that plainly, and records why the bump to 4ba76c18eafe was reverted: the pinned ggml/src/ggml-hrx/loom-jit.cpp still uses the LoomC API that revision removed, so any pairing other than 98d05d94 is unbuildable with ONEBIT_HRX=ON until that file is ported.

No code, pin or workflow changes.

The entry cites 26330cc20490 as the pin engine@main ships; main now pins
e44c9d01a4d5 with hrx-system 98d05d94 (#361 reverted in #363). The study's
measurements still describe what ships: 98d05d94 is the same runtime, and
26330cc20490 is an ancestor of e44c9d01a4d5.
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 9c853ed)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

315 - Partially compliant

Compliant requirements:

  • The PR updates the documentation to reflect that the issue is pre-existing and not introduced by recent changes
  • The PR clarifies the pin note regarding the engine@main state and its relation to the issue

Non-compliant requirements:

  • The PR does not directly fix the underlying HSA memory fault or NaN logits issue
  • The PR does not provide a solution to the runtime-overhead commits that caused the issue

Requires further human verification:

  • Verification of the actual fix for the HSA memory fault and NaN logits issue requires testing on the hardware

361 - Partially compliant

Compliant requirements:

  • The PR correctly documents the bump of both submodules to the AMD-pinned pair
  • The PR clarifies the commits that were rebased onto AMD's pin

Non-compliant requirements:

  • The PR does not actually perform the submodule bumps or commit rebasing

Requires further human verification:

  • The actual submodule bumps and commit rebasing need to be verified in the repository

363 - Partially compliant

Compliant requirements:

  • The PR correctly documents the reason for reverting hrx-system to 98d05d94
  • The PR explains the build issue and how it was resolved

Non-compliant requirements:

  • The PR does not actually perform the revert of hrx-system

Requires further human verification:

  • The actual revert of hrx-system and verification of the build need to be confirmed
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Documentation Clarity

The documentation update in KNOWN-ISSUES.md provides a detailed explanation of the pin changes and their implications, but it does not directly address the root cause of the HSA memory fault or NaN logits issue. The PR is primarily a documentation update and does not introduce any functional changes or fixes to the underlying problem.

**Pin note (2026-10-09).** `engine@main` has since moved twice: first to `third_party/llama.cpp e44c9d01a4d5`
(#347), then to `0e255d774032` (#373, the fork tip after the NVFP4 / Q2_0 / IQ GET_ROWS merges), both with
`third_party/hrx-system 98d05d94`. That is the same runtime the study above ran against, and `26330cc20490` is an
ancestor of both, so the measurements above still describe what ships. Two things learned on the way:
a bump that moved `hrx-system` alone to `4ba76c18eafe` (2026-10-08) left `main` unbuildable with
`ONEBIT_HRX=ON` (the pinned `ggml/src/ggml-hrx/loom-jit.cpp` still used the LoomC API that revision removed; reverted
in #363, and #365 now holds `hrx-system` when LoomC symbols go missing); and with the port of `loom-jit.cpp`
(fork #97) that revision compiles but returns an immediate end-of-text on a 4.7K-token prompt where `98d05d94`
answers (#369), so `hrx-system` stays at `98d05d94` and moving it needs a multi-ubatch prompt check, not a symbol
check. The #373 pin passed that check: needle 20/20 on Qwen3-0.6B.

… long-prompt result

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@bong-water-water-bong bong-water-water-bong changed the title docs: note the engine#315 pin movement in KNOWN-ISSUES.md docs: the engine#315 pin note covers #347, #373 and the hrx-system 4ba76c18 long-prompt result Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 67d5e93

@bong-water-water-bong
bong-water-water-bong enabled auto-merge (squash) October 9, 2026 18:43
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 9c853ed

@bong-water-water-bong
bong-water-water-bong merged commit 9e41ea0 into main Oct 9, 2026
7 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the docs/known-issues-315-pin-note branch October 9, 2026 18:47
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