Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 49 additions & 11 deletions src/llama-hadamard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,30 +129,53 @@ void llama_hadamard::load_keys(llama_model_loader & ml, llm_arch arch) {
const gguf_context * meta = ml.metadata;
uint32_t version = 0;
gguf_bool(meta, "prism.hadamard.tied_output", tied_output);
// The engine's tools/hadamard_q4_0.py stamps onebit.hadamard_q4_0 = 32 instead: every Q4_0 weight of
// the file is rotated by the normalized 32-point Sylvester Walsh-Hadamard matrix per block along

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 4-line comment (and its wrapping) runs against this repo's AGENTS.md (linked from CLAUDE.md), which both caps comments ('Keep code comments concise (usually 1-2 lines)') and forbids fixed-width wrapping ('Avoid hard-wrapping it to a fixed column width - that hurts readability' / 'Do NOT split a line into multiple lines mid-sentence, do NOT try to force the line to fit a fixed number of characters'). The four lines here run 105/101/103/86 characters, the classic shape of wrapping prose to a column rather than letting it run long like the rest of this file's comments do. Worth collapsing to 1-2 unwrapped lines.

// its input dimension, without signs, which is the prism.hadamard transform with block_size 32 and
// sign_mode identity. It names no weights: the Q4_0 tensors are the rotated ones.
bool onebit = false;
if (!gguf_u32(meta, "prism.hadamard.version", version, false)) {
if (tied_output) {
throw std::runtime_error("prism.hadamard.tied_output without prism.hadamard.version");
}
return;
}
if (version != 1 && version != 2) {
const int64_t kid = gguf_find_key(meta, "onebit.hadamard_q4_0");
if (kid < 0) {
return;
}
const gguf_type kt = gguf_get_kv_type(meta, kid);
const int64_t stamp = kt == GGUF_TYPE_INT32 ? gguf_get_val_i32(meta, kid) :

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This re-implements integer-type coercion for the onebit.hadamard_q4_0 stamp instead of reusing gguf_u32() (defined above at line 103), which already covers UINT32/INT32/UINT16/UINT8 for exactly this 'small unsigned stamp' pattern -- it's used for the structurally identical prism.hadamard.block_size key a few lines down. Here only INT32/UINT32 are handled; if the Python writer ever emits this key as UINT8 or UINT16 (plausible for a value as small as 32, and nothing pins the writer to a specific width), stamp falls through to -1 and this throws a confusing 'unsupported onebit.hadamard_q4_0: -1' instead of reading 32. Simpler and more robust as uint32_t stamp32 = 0; if (!gguf_u32(meta, "onebit.hadamard_q4_0", stamp32, false)) return; onebit = stamp32 == 32; if (!onebit) throw ...;.

kt == GGUF_TYPE_UINT32 ? (int64_t) gguf_get_val_u32(meta, kid) : -1;
if (stamp != 32) {
throw std::runtime_error(format("unsupported onebit.hadamard_q4_0: %lld", (long long) stamp));
}
onebit = true;
onebit_q4_0 = true;
} else if (version != 1 && version != 2) {
throw std::runtime_error(format("unsupported prism.hadamard.version: %u", version));
}
if ((version == 2) != tied_output) {
throw std::runtime_error("prism.hadamard version 2 requires tied_output=true; version 1 forbids it");
}
if (tied_output && ml.get_weight("output.weight")) {
if (!onebit && tied_output && ml.get_weight("output.weight")) {
throw std::runtime_error("prism.hadamard.tied_output requires output.weight to be absent");
}

uint32_t block_size = 0;
std::string transform, axis, sign_mode;
std::vector<std::string> weight_names;
gguf_u32(meta, "prism.hadamard.block_size", block_size, true);
gguf_str(meta, "prism.hadamard.transform", transform);
gguf_str(meta, "prism.hadamard.axis", axis);
gguf_str(meta, "prism.hadamard.sign_mode", sign_mode);
gguf_strings(meta, "prism.hadamard.weight_names", weight_names, true);
if (onebit) {
block_size = 32;
transform = "normalized-sylvester-walsh-hadamard";
axis = "input-last-dimension";

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block_size/transform/axis/sign_mode are hardcoded here purely so the generic validation a few lines below (block_size power-of-two check, transform/axis/sign_mode string checks) passes unconditionally for the onebit path -- they can never fail for these fixed values. That validation is dead weight for the onebit branch; skipping it for the onebit case (or restructuring so onebit doesn't flow through checks it always satisfies) would be simpler than fabricating fields to satisfy them.

sign_mode = "identity";
// the names are collected below, from the Q4_0 tensors on a verified matmul path
} else {
gguf_u32(meta, "prism.hadamard.block_size", block_size, true);
gguf_str(meta, "prism.hadamard.transform", transform);
gguf_str(meta, "prism.hadamard.axis", axis);
gguf_str(meta, "prism.hadamard.sign_mode", sign_mode);
gguf_strings(meta, "prism.hadamard.weight_names", weight_names, true);
}

if (block_size == 0 || (block_size & (block_size - 1)) != 0) {
throw std::runtime_error(format("invalid prism.hadamard.block_size: %u", block_size));
Expand All @@ -166,7 +189,7 @@ void llama_hadamard::load_keys(llama_model_loader & ml, llm_arch arch) {
if (sign_mode != "identity" && sign_mode != "explicit") {
throw std::runtime_error(format("unsupported prism.hadamard.sign_mode: %s", sign_mode.c_str()));
}
if (weight_names.empty()) {
if (!onebit && weight_names.empty()) {
throw std::runtime_error("prism.hadamard.weight_names is empty");
}

Expand Down Expand Up @@ -221,7 +244,7 @@ void llama_hadamard::load_keys(llama_model_loader & ml, llm_arch arch) {
"ffn_gate", "ffn_up", "ffn_down",
"ffn_gate_exps", "ffn_up_exps", "ffn_down_exps", "ffn_gate_up_exps",
"ffn_gate_shexp", "ffn_up_shexp", "ffn_down_shexp",
"ssm_out",
"ssm_out", "ssm_alpha", "ssm_beta",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onebit auto-discovery path (added below at the if (onebit) loop) treats this foldable() kinds list as the single source of truth for which weight kinds tools/hadamard_q4_0.py rotated. Nothing in the onebit GGUF stamp cross-checks that assumption (unlike the general prism.hadamard path, which lists weight_names explicitly). ssm_alpha/ssm_beta were added here, but LLM_ARCH_QWEN3NEXT (already in the verified-arch switch above) uses a different, combined tensor blk.N.ssm_ba.weight (LLM_TENSOR_SSM_BETA_ALPHA, routed through build_lora_mm in src/models/qwen3next.cpp) which is not in this list. If a future version of the Python tool ever rotates that tensor for a qwen3next onebit file, it would be silently excluded from rotations here, and verify_graph() cannot catch it either -- it only flags matmuls on weights it already knows are rotated, so a rotated-but-untracked weight is consumed with no activation transform and no error, producing silently wrong output. Worth either keeping this list in lockstep with the Python tool deliberately (a comment/test tying them together), or having the onebit stamp carry an explicit kind/weight list the way the general path does.

};
if (name == "output.weight") {
return true; // the output head goes through build_lora_mm in every arch
Expand All @@ -244,6 +267,18 @@ void llama_hadamard::load_keys(llama_model_loader & ml, llm_arch arch) {
}
return false;
};
if (onebit) {
// tools/hadamard_q4_0.py rotates the Q4_0 matmul weights; a Q4_0 lookup table (token_embd)
// is written in the plain basis and needs no transform
for (const auto & [name, w] : ml.weights_map) {
if (w.tensor->type == GGML_TYPE_Q4_0 && foldable(name)) {
weight_names.push_back(name);
}
}
if (weight_names.empty()) {
throw std::runtime_error("onebit.hadamard_q4_0: the file has no Q4_0 matmul weights");
}
}
for (const auto & name : weight_names) {
if (!foldable(name)) {
throw std::runtime_error(format("prism.hadamard: weight '%s' is not on a verified Hadamard-aware matmul path", name.c_str()));
Expand Down Expand Up @@ -321,6 +356,9 @@ void llama_hadamard::setup(const llama_model & model) {
throw std::runtime_error("prism.hadamard.tied_output is not bound to the token embedding");
}
}
if (!w && onebit_q4_0) {
continue; // a weight of the file the model does not load (the MTP layer without --mtp)
}
if (!w) {
throw std::runtime_error(format("prism.hadamard weight not found: %s", name.c_str()));
}
Expand Down
1 change: 1 addition & 0 deletions src/llama-hadamard.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ struct llama_hadamard {
std::map<uint32_t, std::vector<int32_t>> sign_data; // input width -> signs
bool gdn_v_grouped = false;
bool tied_output = false;
bool onebit_q4_0 = false; // from onebit.hadamard_q4_0: the Q4_0 matmul weights, MTP ones only if loaded

// bound to the model's tensors (setup)
std::unordered_map<const ggml_tensor *, llama_hadamard_transform> rotations;
Expand Down
Loading