Repository navigation
hrx: admit element-wise dense MUL_MAT contractions at input_size % 4 (engine #991) - #208
Conversation
|
#208 ready for review at 21dfef4 Engine ggml-org#991, the padded-tile half — and it turned out to be a contract, not a kernel: no instruction changed. The gate was not where I first looked. The plain dense path declared Measured, three builds, one ticket, and every arm asserted (five padded cases must read
Two things a reviewer should weigh, both in the body: the three motifs are shared with the MoE/gated paths, so their 256-guarantee moves from the JIT to the dispatcher (the coupling I flagged on #204), and my first attempt at this shipped a JIT-time failure that looked like a placement win — the arm printed placement and PPL while the log was full of |
…(engine ggml-org#991) The element-wise weight readers take four values at a time and the activation load is masked with `vector.mask.range [input_k to input_size)`, so a contraction only has to be a multiple of 4 for every packet to stay inside its weight row. The 32-value packet (and the 256 floor) the kernels declared was a contract nobody's arithmetic needed, and it is what leaves the Zyphra ports' dt_proj on the CPU: BlackMamba ssm_dt.weight is [92, 2944] and Zamba v1's is [232, 3712]. - common_mul_mat_supported_plain_dense_input_size: element-wise formats now need input_size >= 4 && % 4 == 0. Block formats are untouched and keep their %256/%128/%64/%32 from common_mul_mat_supported_dense_input_size(). - mul_mat_f32_f32_wmma's ggml.mul_mat.input_size config and the assume in mul_mat_f32_f32_wmma_core and both entry points of mul_mat_f32_f32_decode: range 256 -> 4, multiple_of 32 -> 4. Weakening the contract is safe for every caller: the dispatcher still gates each format at its own granularity, and a block-format contraction is always %256, hence also %4. K % 4 != 0 stays refused on purpose: the last packet of such a row would read up to three values past the row, which is correct by accident only where the following row exists.
21dfef4 to
9972afa
Compare
|
#208 re-based onto Three things changed since the first push, all in the body:
Two harness notes, because they are the same lesson twice: the final arm failed its own assertions on a regex bug in my script (a trailing comma the |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 9972afa1 on the code — the relaxation is scoped to the path whose motifs justify it, and the path it is not applied to is the interesting half. Device row owed.
The decision I most wanted to see is the negative one. common_mul_mat_id_supported_dense_input_size() keeps %32 for element-wise formats, with the reason written down:
// The five MUL_MAT_ID kernels declare `range(256, 32768), multiple_of(32)` … Sharing the looser predicate would let
// an element-wise expert reach a kernel whose own config rejects it, which is a JIT failure, not a fallback.That is exactly the failure mode a contract relaxation invites — the dispatcher and the kernel disagreeing about the same
contraction — and refusing to touch the ID path while relaxing the dense one is the conservative split. Compare it with what
engine #204 did on the ID side last night (which loosened %256 → %32 there, because those kernels declare 32), and the
two together are consistent rather than contradictory.
The relaxation itself is justified in the same place it is made: the element-wise reader loads a 4-value packet and the
caller gates each packet with k < input_size, so a packet starts inside the row exactly when input_size % 4 == 0; the 256
floor and the 256 multiple were a contract, not a requirement of the code. And the change is coherent across every place the
contract is declared — mul_mat_f32_f32_wmma's config, its core's assume, both decode entry points, and all three packet
reader motifs (f16_f16, bf16_f16, f32_f16) — which matters because a config/assume mismatch is a JIT failure, not a slow
path.
What I cannot verify and would not sign off without: test-backend-ops on HRX0 at an input_size that is %4 but not
%32 (the new cases are the guard). A relaxation here either works or produces wrong values, and only the device distinguishes
those; the GPU has been held all night. If the hw-gate label goes on the next pin bump, this is one of the two things worth
pointing it at.
|
Correction to a claim in my standing line — the strict run caught it. I wrote "BlackMamba places completely: 0 of 1179 nodes off HRX, 1 graph split". That is the decode graph only. A So the 512-token prefill ubatch still has the 18 What it means for this PR's claim, narrowly: the Next step, in progress: reproduce it as |
|
Follow-up on the correction above — the reproduction is done and it names the gate, which is not the K contract: At the 512-token prefill ubatch, That means the I pushed |
… it (engine ggml-org#1036 cause 2) The remaining cause of engine ggml-org#1036. #203 stopped storing the shared transformer block once per hybrid layer; this stops storing the per-layer MLP adapter baked into a full FFN matrix, which was the last 1.13 B params / 2.26 GB of the 7B. The duplication was never in the checkpoint: its `gate_up_proj.weight` appears exactly twice (one per shared block) and the per-layer differences live in `gate_up_proj_adapter_list.<visit>.<0|1>`. Our converter merged each layer's rank-128 pair into its own full [n_embd, 2*n_ff] copy. HF applies it as `gate_up_proj(x) + adapter(x)` with no LoRA scaling (modeling_zamba2.py:960-962), on the same normed FFN input and before the gate/up split. So: `gate_up_proj` joins the shared tails (written once per block, aliased by the loader through the mechanism #203 added), and each `gate_up_proj_adapter_list.<visit>.<0|1>` is written as `blk.<layer>.ffn_gate_up_adapter_a/_b.weight`. A new key `zamba2.ffn_gate_up_adapter_rank` both discriminates the two forms (absent/0 = the pre-adapter form) and sizes the tensors. In the graph the pair is applied as base + B*(A*x) through one defaulted parameter on build_ffn, so no other arch's call site changes; the loader, the saver and the arch test all handle both forms, and old GGUFs keep working as a code path rather than a compatibility shim. 2.7B, CPU, one binary for both forms: BF16 6,009,822,848 -> 5,328,903,360 B (-681 MB), tensors 520 -> 531 with ffn_up 9 -> 2 and 9+9 adapters, PPL 16.7937 -> 16.795417 (0.0102 %), KLD 0.000067 +- 0.000002 against the merged file's own logits with same top-p 99.510 %, calibration floor 0.000000, greedy identical. The residual is structural: the merged file has up @ down computed in f32 at conversion time and rounded to bf16, the adapter path applies the pair at run time. Quantized (what gets published): Q5_K_M merged 1994.93 MiB / PPL 17.0430, adapter 1773.11 MiB / PPL 16.9775 - 221.8 MiB smaller and 0.4 % better on the same window. test-llama-archs: zamba2 OK (0.00e+00) + roundtrip OK, with the synthetic model declaring the adapter form so the new tensors are exercised by the saver round-trip. Quantized files need one more thing, found by reading the HRX predicate before the device could say it: the adapter's B contraction has input_size = rank = 128, which the float weight path accepts (input_size >= 4 && % 4 == 0, #208) but the block formats do not (their block layout needs >= 256). So a default Q5_K_M quantized the pair, dropped that matmul on all 9 hybrid layers to the CPU - measured on gfx1151 as 9 of 3051 nodes off HRX and prefill 463.66 vs 589.47 tok/s (0.79x) - while every BF16 arm said "fully on HRX". llama-quant now keeps the pair at F16 by default for 31.8 MiB of a 1804.93 MiB file (explicit --tensor-type still wins, --pure is unchanged), and the resulting file is byte-identical to the one measured strict-clean on the device. Device arms (gfx1151, gpu-hold), BF16 both forms, same binary: GGML_HRX_STRICT=1 exit 0 on all four (prefill and decode, merged and adapter), pp512 441.98 -> 440.57 and tg64 22.87 -> 22.60 (0.997 / 0.988, i.e. no measurable cost), device PPL 19.3635 both.
… it (engine ggml-org#1036 cause 2) The remaining cause of engine ggml-org#1036. #203 stopped storing the shared transformer block once per hybrid layer; this stops storing the per-layer MLP adapter baked into a full FFN matrix, which was the last 1.13 B params / 2.26 GB of the 7B. The duplication was never in the checkpoint: its `gate_up_proj.weight` appears exactly twice (one per shared block) and the per-layer differences live in `gate_up_proj_adapter_list.<visit>.<0|1>`. Our converter merged each layer's rank-128 pair into its own full [n_embd, 2*n_ff] copy. HF applies it as `gate_up_proj(x) + adapter(x)` with no LoRA scaling (modeling_zamba2.py:960-962), on the same normed FFN input and before the gate/up split. So: `gate_up_proj` joins the shared tails (written once per block, aliased by the loader through the mechanism #203 added), and each `gate_up_proj_adapter_list.<visit>.<0|1>` is written as `blk.<layer>.ffn_gate_up_adapter_a/_b.weight`. A new key `zamba2.ffn_gate_up_adapter_rank` both discriminates the two forms (absent/0 = the pre-adapter form) and sizes the tensors. In the graph the pair is applied as base + B*(A*x) through one defaulted parameter on build_ffn, so no other arch's call site changes; the loader, the saver and the arch test all handle both forms, and old GGUFs keep working as a code path rather than a compatibility shim. 2.7B, CPU, one binary for both forms: BF16 6,009,822,848 -> 5,328,903,360 B (-681 MB), tensors 520 -> 531 with ffn_up 9 -> 2 and 9+9 adapters, PPL 16.7937 -> 16.795417 (0.0102 %), KLD 0.000067 +- 0.000002 against the merged file's own logits with same top-p 99.510 %, calibration floor 0.000000, greedy identical. The residual is structural: the merged file has up @ down computed in f32 at conversion time and rounded to bf16, the adapter path applies the pair at run time. Quantized (what gets published): Q5_K_M merged 1994.93 MiB / PPL 17.0430, adapter 1773.11 MiB / PPL 16.9775 - 221.8 MiB smaller and 0.4 % better on the same window. test-llama-archs: zamba2 OK (0.00e+00) + roundtrip OK, with the synthetic model declaring the adapter form so the new tensors are exercised by the saver round-trip. Quantized files need one more thing, found by reading the HRX predicate before the device could say it: the adapter's B contraction has input_size = rank = 128, which the float weight path accepts (input_size >= 4 && % 4 == 0, #208) but the block formats do not (their block layout needs >= 256). So a default Q5_K_M quantized the pair, dropped that matmul on all 9 hybrid layers to the CPU - measured on gfx1151 as 9 of 3051 nodes off HRX and prefill 463.66 vs 589.47 tok/s (0.79x) - while every BF16 arm said "fully on HRX". llama-quant now keeps the pair at F16 by default for 31.8 MiB of a 1804.93 MiB file (explicit --tensor-type still wins, --pure is unchanged), and the resulting file is byte-identical to the one measured strict-clean on the device. Device arms (gfx1151, gpu-hold), BF16 both forms, same binary: GGML_HRX_STRICT=1 exit 0 on all four (prefill and decode, merged and adapter), pp512 441.98 -> 440.57 and tg64 22.87 -> 22.60 (0.997 / 0.988, i.e. no measurable cost), device PPL 19.3635 both.
… it (engine ggml-org#1036 cause 2) (#218) The remaining cause of engine ggml-org#1036. #203 stopped storing the shared transformer block once per hybrid layer; this stops storing the per-layer MLP adapter baked into a full FFN matrix, which was the last 1.13 B params / 2.26 GB of the 7B. The duplication was never in the checkpoint: its `gate_up_proj.weight` appears exactly twice (one per shared block) and the per-layer differences live in `gate_up_proj_adapter_list.<visit>.<0|1>`. Our converter merged each layer's rank-128 pair into its own full [n_embd, 2*n_ff] copy. HF applies it as `gate_up_proj(x) + adapter(x)` with no LoRA scaling (modeling_zamba2.py:960-962), on the same normed FFN input and before the gate/up split. So: `gate_up_proj` joins the shared tails (written once per block, aliased by the loader through the mechanism #203 added), and each `gate_up_proj_adapter_list.<visit>.<0|1>` is written as `blk.<layer>.ffn_gate_up_adapter_a/_b.weight`. A new key `zamba2.ffn_gate_up_adapter_rank` both discriminates the two forms (absent/0 = the pre-adapter form) and sizes the tensors. In the graph the pair is applied as base + B*(A*x) through one defaulted parameter on build_ffn, so no other arch's call site changes; the loader, the saver and the arch test all handle both forms, and old GGUFs keep working as a code path rather than a compatibility shim. 2.7B, CPU, one binary for both forms: BF16 6,009,822,848 -> 5,328,903,360 B (-681 MB), tensors 520 -> 531 with ffn_up 9 -> 2 and 9+9 adapters, PPL 16.7937 -> 16.795417 (0.0102 %), KLD 0.000067 +- 0.000002 against the merged file's own logits with same top-p 99.510 %, calibration floor 0.000000, greedy identical. The residual is structural: the merged file has up @ down computed in f32 at conversion time and rounded to bf16, the adapter path applies the pair at run time. Quantized (what gets published): Q5_K_M merged 1994.93 MiB / PPL 17.0430, adapter 1773.11 MiB / PPL 16.9775 - 221.8 MiB smaller and 0.4 % better on the same window. test-llama-archs: zamba2 OK (0.00e+00) + roundtrip OK, with the synthetic model declaring the adapter form so the new tensors are exercised by the saver round-trip. Quantized files need one more thing, found by reading the HRX predicate before the device could say it: the adapter's B contraction has input_size = rank = 128, which the float weight path accepts (input_size >= 4 && % 4 == 0, #208) but the block formats do not (their block layout needs >= 256). So a default Q5_K_M quantized the pair, dropped that matmul on all 9 hybrid layers to the CPU - measured on gfx1151 as 9 of 3051 nodes off HRX and prefill 463.66 vs 589.47 tok/s (0.79x) - while every BF16 arm said "fully on HRX". llama-quant now keeps the pair at F16 by default for 31.8 MiB of a 1804.93 MiB file (explicit --tensor-type still wins, --pure is unchanged), and the resulting file is byte-identical to the one measured strict-clean on the device. Device arms (gfx1151, gpu-hold), BF16 both forms, same binary: GGML_HRX_STRICT=1 exit 0 on all four (prefill and decode, merged and adapter), pp512 441.98 -> 440.57 and tg64 22.87 -> 22.60 (0.997 / 0.988, i.e. no measurable cost), device PPL 19.3635 both. Co-authored-by: agent <agent@local>
hrx: admit element-wise dense MUL_MAT contractions at input_size % 4 (engine ggml-org#991)
The gap
The plain dense
MUL_MATpath declared its contraction asrange(input_size, 256, 32768), multiple_of(input_size, 32)—in
mul_mat_f32_f32_wmma'sggml.mul_mat.input_sizeconfig, in the assume of its core, in both entry points ofmul_mat_f32_f32_decode, and (this is the one that actually bites) in the element-wise packet reader motifsf16_f16/bf16_f16/f32_f16, each of which buildsview<[bounded_input_size]x{bf16,f16,f32}>under a 256 contract.common_mul_mat_supported_plain_dense_input_sizerepeated the same>= 256 && % 32 == 0for element-wise weights. So anycontraction that is not 32-aligned, or is narrower than 256, went to the CPU.
That is the last class in the Zyphra ports: BlackMamba's
ssm_dt.weightis[92, 2944], Zamba v1's is[232, 3712].Why % 4 is the real requirement
Reading the kernels and the motifs, not the contract:
vector.loadis unmasked — but its callergates each packet with
k < input_size, so the four values are inside the row exactly wheninput_size % 4 == 0;vector.mask.range [input_k to input_size), i.e. zero-exact at element granularity;padded_input_size = input_size + 255,block_count,valid_block,safe_block = select(valid_block, block, 0)andselect(valid_block, sum, 0)on the contribution.32 and 256 were contracts nobody's arithmetic needed.
K % 4 != 0stays refused on purpose — such a row's last packetwould read up to three values past the row, which is correct only by accident of where the next row happens to be. A test
case asserts that boundary.
What changes
common_mul_mat_supported_plain_dense_input_size: element-wise formats needinput_size >= 4 && % 4 == 0. Blockformats are untouched and keep their
%256/%128/%64/%32fromcommon_mul_mat_supported_dense_input_size().range 256 -> 4,multiple_of 32/256 -> 4in the five places above. Weakening a declared contract is safe for everyexisting caller: a block-format contraction is always a multiple of 256, hence also of 4.
Who enforces what, after this. The three motifs are shared with the MoE/gated paths, so for those paths the 256
guarantee moves from the JIT to the dispatcher (they stay gated at
%32/%256). That is a real coupling — the same one Iflagged on #204: a predicate that grows looser in one path changes what can reach another path's kernels. Here the only new
exposure is a shape no dispatcher admits today, and the motif's arithmetic is caller-gated per packet, so the change is
safe; it is stated rather than left implicit.
Evidence
Three builds, one ticket, and every arm asserted rather than printed: the five padded cases must read
OK, thek=90control must read
not supported, no arm may logLoom compilation failed, and every model arm must emit its bench row andits PPL estimate.
failed=0.test-backend-ops -b HRX0 -o MUL_MAT(this branch, before#207):BlackMamba-2.8B (this change alone; the
basearm is the same binary tree with the batched dispatch disabled, i.e.origin behaviour for
MUL_MAT):-r 5)-r 5)dt_proj's 18 nodes are gone, and the remaining 72 are the MoE classes that fork #204 covers. PPL is unchanged at theprecision printed; the per-shape correctness evidence is the
test-backend-opstable.Zamba-7B-v1 needs this and #207, because its
dt_projnodes are also batched (ne[2] = 2):-r 5)-r 5)The middle row is the honest one for this PR: it moves v1 not at all, because a batched node is still refused before any
size rule. The last row is measured on a local merge of this branch with #207 (
git merge 1f2768431, never pushed) sothe reviewer can see what lands once both are in; #207 must merge first for that number to apply.
What this does not do
K % 4 != 0stays on the CPU, with a test asserting it.mul_mat_id_swiglu_f32_f32_wmma,mul_mat_id_decode_f32andmul_mat_dual_q4_f32_decodekeep their%32contracts —they belong to the MoE/gated classes, and widening them without a shape that needs it would be an unmeasured change.
Notes for the reviewer
failure as a placement win. I widened the predicate and the op/core/decode assumes, and the arm happily printed a
placement line (
72 off,dt_projgone) and a PPL — while the log was full ofLoom compilation failed … predicate 'range' on 'bounded_input_size' requires range [256, 32768]. The contract thatactually rejected the spec lives in the three motifs, not in the files I had read; the verdict fields in my greps were
blank, and I read blank as printed. Take 2 asserts on the evidence instead of printing it, which is why the table above
has
failed=0next to it rather than a summary line.device-vs-CPU rows plus the model placement.