Skip to content

Pin ROCmFPX fc664ba: Hadamard rotation per tensor, any drafter type beside a rotated file - #262

Merged
bong-water-water-bong merged 1 commit into
mainfrom
hadamard-per-tensor
Oct 1, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
hadamard-per-tensor

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

ROCmFPX#7 makes the Hadamard activation rotation per tensor: the loader flags
the Q4_0 weights of a file stamped onebit.hadamard_q4_0 = 32 and only those get
rotated activations. A plain Q4_0 DFlash2 drafter beside Qwen3.8-27B-H32 now
accepts 216/263 drafts on code at 45.6 tok/s (Q8_0: 217/260, 44.4); with the old
process-wide GGML_Q4_0_HADAMARD it accepted 0/337.

serve no longer sets GGML_Q4_0_HADAMARD (the backend reads the stamp; a value in
the environment still forces the old process-wide rotation), and the #259
guard that refused a Q4_0 drafter goes, with gguf_tensor_type_count.
hadamard_route.sh: no process-wide variable, Q4_0 and Q8_0 drafters both
accepted; long_route.sh follows. docs/lean.md, serve.md and
tools/hadamard_q4_0.py describe the per-tensor rotation.

🤖 Generated with Claude Code

…eside a rotated file

ROCmFPX#7 makes the Hadamard activation rotation per tensor: the loader flags
the Q4_0 weights of a file stamped onebit.hadamard_q4_0 = 32 and only those get
rotated activations. A plain Q4_0 DFlash2 drafter beside Qwen3.8-27B-H32 now
accepts 216/263 drafts on code at 45.6 tok/s (Q8_0: 217/260, 44.4); with the old
process-wide GGML_Q4_0_HADAMARD it accepted 0/337.

serve no longer sets GGML_Q4_0_HADAMARD (the backend reads the stamp; a value in
the environment still forces the old process-wide rotation), and the #259
guard that refused a Q4_0 drafter goes, with gguf_tensor_type_count.
hadamard_route.sh: no process-wide variable, Q4_0 and Q8_0 drafters both
accepted; long_route.sh follows. docs/lean.md, serve.md and
tools/hadamard_q4_0.py describe the per-tensor rotation.

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

context7 Bot commented Oct 1, 2026

Copy link
Copy Markdown

Docs7 for 1bit-monster/engine

Result Status Action
Deployment ➖ Not used —
Content review ✅ Passed. No problems found. View findings

Commit 54729f5

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

7 - Partially compliant

Compliant requirements:

  • Pin ROCm/hrx and ROCm/hrx-system with correct commits
  • Use llama.cpp fork with correct branch
  • Apply patches for build plumbing and runtime fix
  • Build with -DONEBIT_HRX=ON and clean submodules
  • Wire Lemonade to use llamacpp-hrx on HRX20 and llamacpp/Vulkan on Vulkan0
  • Add hrx_device option with default HRX0
  • Verify reproducible kernels and speed parity
  • Test end-to-end with tests/hrx_lemonade_e2e.sh
  • Default build unchanged with smoke test passing

Non-compliant requirements:

  • No relocatable package (HRX libraries load via build-tree RPATHs)
  • No HRX in CI (runner has no AMD GPU)

Requires further human verification:

  • Verification of relocatable package behavior
  • CI setup for HRX testing

259 - Partially compliant

Compliant requirements:

  • Added gguf_tensor_type_count function in app/gguf_meta.h
  • serve now checks --dflash and --mtp files against tensor type count
  • Updated docs/serve.md to reflect that any drafter type works next to a Hadamard-rotated file
  • Tests pass with Q4_0 drafter accepted and Q8_0 drafter accepted

Non-compliant requirements:

  • No mention of model registry from step 5 (needed for zaya Q4NX through Lemonade)

Requires further human verification:

  • Verification that zaya Q4NX works through Lemonade (model registry requirement)
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Process-wide Hadamard flag removal

The PR removes the process-wide GGML_Q4_0_HADAMARD=1 environment variable setting that was previously used to apply Hadamard rotation to all Q4_0 matmuls in the backend. This change is necessary because the new per-tensor Hadamard rotation approach stamps Q4_0 weights in the GGUF file, and only those weights are rotated. The backend now reads this stamp and applies rotation only to the appropriate tensors, allowing plain Q4_0 drafters to work alongside Hadamard-rotated files. This change improves compatibility and performance by avoiding unnecessary rotations.

if (hadamard_q4_0(o.model)) {
    // tools/hadamard_q4_0.py stamped it: its Q4_0 weights are rotated. The lean ROCm build sees
    // the stamp and rotates the activations of those weights, and of no others, so a plain
    // Q4_0 drafter or MTP head works beside it (ROCmFPX#7); every Q4_0 matmul takes the W4A4
    // kernel (docs/lean.md). A value set in the environment wins (GGML_W4A4_TENSORS= keeps
    // exact int8).
    if (device != "rocm") throw std::runtime_error(o.model + " is Hadamard-rotated: it runs on --device rocm only");
    if (!std::getenv("GGML_W4A4_TENSORS")) env.push_back("GGML_W4A4_TENSORS=all");
    // micro-batch size: recipe rotated-moe-ub1024 (config/recipes.json)
}
Test update for new Hadamard behavior

The test script hadamard_route.sh has been updated to reflect the new behavior where a plain Q4_0 drafter is now accepted next to a Hadamard-rotated file. Previously, the test would have refused such a configuration due to the process-wide Hadamard flag. The test now validates that both Q4_0 and Q8_0 drafters are accepted, and checks that the GGML_Q4_0_HADAMARD environment variable is not set in the backend environment, confirming the per-tensor approach.

check "  without a process-wide GGML_Q4_0_HADAMARD" '[ "$(field "$scratch/h32.json" "r[\"env\"].get(\"GGML_Q4_0_HADAMARD\")")" = None ]'
check "  and GGML_W4A4_TENSORS=all" '[ "$(field "$scratch/h32.json" "r[\"env\"].get(\"GGML_W4A4_TENSORS\")")" = all ]'
check "  dense: llama-server's own micro-batch" '[ "$(field "$scratch/h32.json" "\"-ub\" in r[\"argv\"]")" = False ]'

run "$scratch/h32moe.gguf" "$scratch/h32moe.json"
check "a rotated MoE file gets 1024-token micro-batches" '[ "$(field "$scratch/h32moe.json" "r[\"argv\"][r[\"argv\"].index(\"-ub\")+1]")" = 1024 ]'

run "$scratch/plain.gguf" "$scratch/plain.json"
check "an unstamped file still goes to Vulkan0" '[ "$(field "$scratch/plain.json" "r[\"argv\"][r[\"argv\"].index(\"--device\")+1]")" = Vulkan0 ]'
check "  with neither variable" '[ "$(field "$scratch/plain.json" "sorted(r[\"env\"])")" = "[]" ]'

run "$scratch/h32.gguf" "$scratch/df.json" --dflash "$scratch/draft.gguf"
after() { field "$scratch/df.json" "r[\"argv\"][r[\"argv\"].index(\"$1\")+1]"; }
check "--dflash on a rotated file stays on ROCm0" '[ "$(after --device)" = ROCm0 ]'
check "  with the DFlash drafter" '[ "$(after --spec-type)" = draft-dflash ] && [ "$(after -md)" = "$scratch/draft.gguf" ]'
check "  n-max 16 without a block size, p-min 0.4" '[ "$(after --spec-draft-n-max)" = 16 ] && [ "$(after --spec-draft-p-min)" = 0.4 ]'
check "  and the Hadamard W4A4 environment" '[ "$(field "$scratch/df.json" "r[\"env\"].get(\"GGML_W4A4_TENSORS\")")" = all ]'

run "$scratch/h32.gguf" "$scratch/df8.json" --dflash "$scratch/draft8.gguf"
check "--dflash with a block-8 drafter drafts 7" '[ "$(field "$scratch/df8.json" "r[\"argv\"][r[\"argv\"].index(\"--spec-draft-n-max\")+1]")" = 7 ]'

run "$scratch/h32.gguf" "$scratch/dfq4.json" --dflash "$scratch/draftq4.gguf"
check "a Q4_0 drafter next to a rotated file is accepted" '[ "$(field "$scratch/dfq4.json" "r[\"argv\"][r[\"argv\"].index(\"-md\")+1]")" = "$scratch/draftq4.gguf" ]'
run "$scratch/h32.gguf" "$scratch/dfq8.json" --dflash "$scratch/draftq8.gguf"
check "  so is a Q8_0 one" '[ "$(field "$scratch/dfq8.json" "r[\"argv\"][r[\"argv\"].index(\"-md\")+1]")" = "$scratch/draftq8.gguf" ]'

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

Review (PR-Agent duty). Looks good.

  • The pin fc664ba is the tip of ROCmFPX 1bit/vulkan-rocmi4 (checked with ls-remote) and is the merge of ROCmFPX#7.
  • Not setting GGML_Q4_0_HADAMARD is the right default now that the backend reads the stamp, and keeping a value set in the environment as an override is sensible.
  • Dropping the #259 guard and gguf_tensor_type_count is consistent with that: the per-tensor flag makes a plain Q4_0 drafter correct, and the PR has the numbers (216/263 accepted, 45.6 tok/s).
  • The tests now cover both drafter types, and checks pass.

One non-blocking note: serve and the ROCm llama-server have to come from the same build. A ROCm llama-server built before ROCmFPX#7 and paired with this serve gets no rotation at all, and its output is silently wrong rather than refused. Since the engine build produces both from the pinned submodule, that only happens with a mixed install. A version stamp check (or setting the env when the backend predates the flag) would close it someday.

Merging.

@bong-water-water-bong
bong-water-water-bong merged commit 7c13e28 into main Oct 1, 2026
11 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the hadamard-per-tensor branch October 1, 2026 15:54
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