Skip to content

hrx: scalar-scatter SET_ROWS for the non-FA path (-fa off) (#95) - #106

Merged
bong-water-water-bong merged 2 commits into
mainfrom
fix/issue-95-hrx-nonfa-scatter
Sep 26, 2026
Merged

bong-water-water-bong merged 2 commits into
mainfrom
fix/issue-95-hrx-nonfa-scatter

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Follow-up to #104. The default FA-on path is correct, but an explicit -fa off (the non-FA attention path) still failed with unsupported HRX node SET_ROWS.

Root cause

The non-FA path flattens the K/V rows before the KV-cache write, so llama.cpp emits SET_ROWS element-wise (hidden_size == 1): each value is scattered to its own cache row. The row-wise SET_ROWS dispatcher only supports hidden_size 4..32768 (vector4), so it rejected the element-wise shape and the graph failed.

Fix (in 1bit-MONSTER/llama.cpp@c35833404)

Add a scalar-scatter kernel (ggml_set_rows_scatter) and a hidden_size == 1 matcher that dispatches the element-wise write: for each value i, cache[indices[i]] = f16(rows[i]).

Verification

  • -fa off now answers "Paris" on Qwen3-Coder-30B-A3B-Instruct Q4_K_M and GLM-4.7-Flash Q4_K_M (previously 500).
  • tests/serve_e2e.sh answers "Paris" on Qwen3-Coder-30B-A3B, GLM-4.7-Flash and Qwen3.6-35B-A3B Q8_0 (FA-on, no regression).

Bump llama.cpp to c35833404, which dispatches the non-FA path's element-wise
KV-cache write (SET_ROWS with hidden_size 1) to a scalar scatter kernel, so an
explicit -fa off no longer fails with 'unsupported HRX node SET_ROWS'.
@context7

context7 Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Docs7 for 1bit-monster/engine

Result Status Action
Deployment ➖ Not used —
Content review ➖ Did not run. This site has no agent runs available this month. Wait for the monthly reset or check your Docs7 plan. —

Commit b66e8b5

@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit b66e8b5)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

104 - Partially compliant

Compliant requirements:

  • Pin third_party/llama.cpp to commit 96f6b89
  • Fix GLM-4.7-Flash and Qwen3-Coder at full context
  • Remove 32768 context default for --device hrx
  • Update docs (registry.md, PORTING and serve.md) and registry/checked.json
  • Verify on Strix Halo with fresh build (-DONEBIT_HRX=ON)
  • Ensure 7 of 7 HRX tests pass at full context
  • Ensure serve_e2e passes with vulkan --prefill-device hrx
  • Ensure test-backend-ops -b HRX0 passes with 790/790

Non-compliant requirements:

  • None

Requires further human verification:

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

Flash Attention Flag Handling

The new -fa flag is added to control flash attention behavior, but the handling logic in launch_for function may not correctly propagate this flag when --prefill-device is used. Specifically, when split is true, the -fa on is hardcoded, which may override the user's explicit -fa off setting. This could lead to inconsistent behavior between the prefill and generation devices.

if (!o.flash_attn.empty() && !split) { argv.insert(argv.end(), {"-fa", o.flash_attn}); }
if (split) {
    argv.insert(argv.end(), {"-fa", "on"});

So an explicit -fa off reaches the HRX build and exercises the non-FA
attention path, instead of 1bit serve rejecting the unknown option.
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit b66e8b5

@bong-water-water-bong
bong-water-water-bong merged commit 0872c5b into main Sep 26, 2026
3 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the fix/issue-95-hrx-nonfa-scatter branch September 26, 2026 00:41
bong-water-water-bong pushed a commit that referenced this pull request Sep 26, 2026
…ama.cpp #2, #5, #6)

The pin adds the zaya architecture and its converter (#2, #5) on top of #104's #95 fixes, and
carries #106's scalar-scatter SET_ROWS onto the tracked branch (#6): #106 pinned c358334,
which is on no branch of the fork. registry/architectures.json, regenerated:
ZayaForCausalLM maps to hrx and vulkan (266 architectures, all mapped).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bong-water-water-bong added a commit that referenced this pull request Sep 26, 2026
…nly architectures (#107)

* ZAYA1 (Zyphra) on Vulkan: serve routes fork-only architectures to our llama.cpp

`1bit serve --device vulkan` reads the GGUF's general.architecture (app/gguf_meta.h) and
runs an architecture only our llama.cpp implements (zaya) on the HRX build's Vulkan0, even
when the upstream Vulkan build is present. registry_build counts such architectures for
vulkan the same way, and check_models.tsv checks ZAYA1-8B Q4_K_M on vulkan.

docs/vulkan.md: converting and serving ZAYA1, with the checks against transformers and
the measured speeds on Strix Halo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ZAYA1 on HRX and ROCm: docs, and serve_e2e on vulkan and hrx in the registry

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Pin llama.cpp 5556bf2: ZAYA1 on Vulkan, HRX and ROCm (1bit-MONSTER/llama.cpp #2, #5, #6)

The pin adds the zaya architecture and its converter (#2, #5) on top of #104's #95 fixes, and
carries #106's scalar-scatter SET_ROWS onto the tracked branch (#6): #106 pinned c358334,
which is on no branch of the fork. registry/architectures.json, regenerated:
ZayaForCausalLM maps to hrx and vulkan (266 architectures, all mapped).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* registry: vulkan and hrx rows re-checked at b9812e0 (16/16 pass, ZAYA1-8B included)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: build gguf_meta_test

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
bong-water-water-bong added a commit that referenced this pull request Sep 26, 2026
…n the model rows (#136)

- pins_on_branch, before the build: every pin a PR moves must be on the branch .gitmodules
  names for that submodule (GitHub compare: identical or ahead). Twice this week an engine PR
  pinned a fork commit on no branch (#103, #106); the shallow clones follow the branch and
  could not have fetched it.
- bump-hrx/* and bump-llama-vulkan/*: every hrx / vulkan row of registry/check_models.tsv
  through registry_check (serve end to end per architecture: ZAYA1, the Qwen MoE models,
  GLM, MiniCPM), under the memory guard; any FAIL holds the bump. Build + ctest alone serve
  one small GGUF, which would not notice an architecture breaking.

Tested pins_on_branch against the open PRs: #121's llama.cpp pin f59f6d37 fails (diverged from
1bit/hrx-vulkan-patched); the ZINC (#132) and Lemonade (#133) pins pass.

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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