Skip to content

serve: refuse a Q4_0 drafter next to a Hadamard-rotated file - #259

Merged
bong-water-water-bong merged 1 commit into
mainfrom
hadamard-drafter-guard
Oct 1, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
hadamard-drafter-guard

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Next to a Hadamard-rotated file, 1bit serve now refuses a drafter or MTP head that holds plain Q4_0 tensors, with an error that says why.

Why: GGML_Q4_0_HADAMARD applies to every Q4_0 matmul in the backend process, the drafter's included. A plain-Q4_0 drafter would read its activations rotated and accept nothing. The leaderboard run hit exactly this with a Q4_0 DFlash2 drafter.

What changes:

  • app/gguf_meta.h gains gguf_tensor_type_count, which reads only the GGUF header and tensor infos.
  • serve checks --dflash and --mtp files against it.
  • docs/serve.md says so.

Tests: tests/hadamard_route.sh passes 15/15, including a Q4_0 drafter (refused) and a Q8_0 one (accepted). I also checked it against a real 27B Q4_0 file, which is refused, and against the real Q8_0 DFlash2 drafter, which is accepted.

🤖 Generated with Claude Code

GGML_Q4_0_HADAMARD rotates the activations of every Q4_0 matmul in the ROCm
backend, the drafter's included, so a drafter or MTP head with plain Q4_0
tensors reads its activations rotated and accepts no drafts (found by the
leaderboard run with a Q4_0 DFlash2 drafter). serve now refuses it with the
reason. app/gguf_meta.h gets gguf_tensor_type_count (reads the key/value
section and tensor infos only). hadamard_route.sh: a Q4_0 drafter is refused,
a Q8_0 one is not (15/15); checked on a real 27B Q4_0 file too.

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 e2248c6

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

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

Incorrect tensor type check

The code checks for Q4_0 tensors in draft files using gguf_tensor_type_count(draft, 2), but the function gguf_tensor_type_count is designed to count tensors of a specific GGML type. The check gguf_tensor_type_count(draft, 2) > 0 will return true if any tensor of type 2 (Q4_0) exists in the file, but it does not verify if the tensor is actually Hadamard-rotated. This could lead to false positives where a non-Hadamard-rotated Q4_0 tensor is incorrectly accepted.

if (!draft.empty() && gguf_tensor_type_count(draft, 2) > 0 && !hadamard_q4_0(draft))
Potential buffer overflow in skip_str

The function skip_str uses n > (1u << 20) to check for oversized strings, but this check is not sufficient to prevent potential buffer overflows. If n is close to the maximum value of uint64_t, the subsequent f.seekg(static_cast<std::streamoff>(n), std::ios::cur) could cause undefined behavior or overflow. The check should be more robust to prevent such issues.

if (!rd(&n, 8) || n > (1u << 20)) return false;

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

  • gguf_tensor_type_count follows the GGUF layout: the KV scalar sizes match types 0-12, string arrays are walked, and each tensor info is name, n_dims, 8-byte dims, u32 type, u64 offset. Anything unreadable returns -1, so the check is skipped rather than failing a launch on an odd file.
  • A 1 MiB cap per string is safe: chat templates and vocab entries are far below it, and walking 248k vocab strings is header-only work.
  • The rule is the right one. The rotation is process-wide (GGML_Q4_0_HADAMARD), so plain Q4_0 in the drafter or MTP file can't be read correctly, and a rotated drafter is still allowed through hadamard_q4_0(draft).
  • The test covers both sides (Q4_0 refused, Q8_0 accepted), the docs are updated, and checks pass.

Merging.

@bong-water-water-bong
bong-water-water-bong merged commit 940b5d8 into main Oct 1, 2026
11 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the hadamard-drafter-guard branch October 1, 2026 12:59
bong-water-water-bong added a commit that referenced this pull request Oct 1, 2026
…eside a rotated file (#262)

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: 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant