From 8f4d5b49b1937b1c239401d36a99c7bbfba4bc76 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 19:08:08 -0400 Subject: [PATCH 1/5] jouleclaw-loader-gguf: 161 -> 81 clippy warnings, and what the gate still needs #373 left this crate as the remaining half of the per-configuration lint hole: `jouleclaw-omni --features metal` is gated with `-D warnings`, but loader-gguf, which omni depends on, is not linted at all on its own. Two steps here, neither of them risky: - `cargo clippy --fix` applied the machine-applicable suggestions. Only seven files; most of this crate's lints are not auto-fixable. - the same three policy decisions omni's lib.rs already records, for the same reasons, since this is quantisation and tokeniser code with the same shapes: `needless_range_loop` (kernels walk several arrays at one index, often at an offset), `too_many_arguments` (a dequantiser's signature is its block format), and the two doc-indentation lints. 161 -> 81 on the default feature set. It still cannot join the `-D warnings` list, and the measurement says precisely why. Counting distinct sites, deduplicated, by crate: default + rayon fabric-meter loader-gguf src 75 76 loader-gguf tests/examples 10 14 jouleclaw-backend-apple - 43 Three things that measurement settles, all of which I had wrong at some point while taking it: - `--no-deps` does NOT exclude workspace path-dependencies. Running `clippy -p jouleclaw-loader-gguf` reports jouleclaw-backend-apple's warnings too, so gating this crate means gating that one as well. That is also why 27 `unused_unsafe` and 15 `unused_imports` appear under fabric-meter and not on default: they are backend-apple's, in mps_matmul.rs, not this crate's. - the remainder is in `src`, not tests. I guessed "largely tests and examples" from the first number and it is 75 against 10. - what is left is mechanical rather than policy: 16 `chunks_exact_to_as_chunks`, 9 `unnecessary_cast`, 9 `redundant_guards`, 6 `collapsible_if`, 6 `manual_repeat_n`, 5 `manual_is_multiple_of`, 5 `unnecessary_sort_by`, and a tail of ones and twos. So closing it is ~80 edits here plus ~43 in a second crate, each needing care because `chunks_exact` -> `as_chunks` changes remainder handling in quantisation code. That is a decision about appetite, not a blocker, and it is left as one. Verified: 343 loader-gguf tests, the fabric-meter `process_calibration` build that caught a deleted label yesterday, and omni's own `-D warnings` gate and 77 tests all still pass. Co-Authored-By: Claude Opus 5 --- .../benches/matmul_q56_k.rs | 2 +- .../benches/matmul_q8_0.rs | 4 ++-- .../examples/load_safetensors.rs | 4 +--- .../crates/jouleclaw-loader-gguf/src/lib.rs | 18 ++++++++++++++++++ .../crates/jouleclaw-loader-gguf/tests/bpe.rs | 2 +- .../jouleclaw-loader-gguf/tests/bpe_regex.rs | 2 +- .../jouleclaw-loader-gguf/tests/dequant.rs | 2 +- .../jouleclaw-loader-gguf/tests/tokenizer.rs | 2 +- 8 files changed, 26 insertions(+), 10 deletions(-) diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q56_k.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q56_k.rs index 287508861..20ff87e63 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q56_k.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q56_k.rs @@ -61,7 +61,7 @@ fn bench_q5_k(c: &mut Criterion) { g.finish(); let mut probe_out = vec![0f32; 1]; if matmul_q5_k_amx_tiled( - &vec![0f32; 256], &vec![0u8; Q5_K_BLOCK_BYTES], + &vec![0f32; 256], &[0u8; Q5_K_BLOCK_BYTES], &[], 1, 1, 256, &mut probe_out, ).is_err() { return; } let mut g = c.benchmark_group("matmul_q5_k_amx_tiled"); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q8_0.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q8_0.rs index a11b82da0..7895c46be 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q8_0.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q8_0.rs @@ -273,7 +273,7 @@ fn bench_matmul_q8_0_threaded(c: &mut Criterion) { // Skip cleanly when rayon feature is off. let mut probe_out = vec![0f32; 1]; if matmul_q8_0_threaded( - &vec![0f32; 32], &vec![0u8; Q8_0_BLOCK_BYTES], &[], + &[0f32; 32], &[0u8; Q8_0_BLOCK_BYTES], &[], 1, 1, 32, &mut probe_out, ) .is_err() @@ -323,7 +323,7 @@ fn bench_matmul_q8_0_amx_tiled_threaded(c: &mut Criterion) { use jouleclaw_loader_gguf::matmul_q8_0::matmul_q8_0_amx_tiled_threaded; let mut probe_out = vec![0f32; 1]; if matmul_q8_0_amx_tiled_threaded( - &vec![0f32; 32], &vec![0u8; Q8_0_BLOCK_BYTES], &[], + &[0f32; 32], &[0u8; Q8_0_BLOCK_BYTES], &[], 1, 1, 32, &mut probe_out, ) .is_err() diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_safetensors.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_safetensors.rs index d6889774a..9af45a78d 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_safetensors.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_safetensors.rs @@ -5,9 +5,7 @@ //! cargo run --release --example load_safetensors -- path/to/model.safetensors //! ``` -use jouleclaw_loader_gguf::safetensors_reader::{ - read_safetensors_file, SafetensorsDtype, -}; +use jouleclaw_loader_gguf::safetensors_reader::read_safetensors_file; use std::collections::BTreeMap; use std::path::PathBuf; use std::time::Instant; diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs index 3952409f2..9eae89ef9 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs @@ -16,6 +16,24 @@ // handle, the path a store was opened from), and a few helpers are written // ahead of the path that will call them. #![allow(dead_code)] +// +// ── pedantic/style lints this crate has decided against ───────────────────── +// +// The same three decisions jouleclaw-omni's lib.rs records, for the same +// reasons: this is quantisation and tokeniser code with the same shapes. +// +// Quantised kernels walk several arrays at one index, often at an offset from +// the loop variable, which iterator form cannot express without obscuring +// which index is read. +#![allow(clippy::needless_range_loop)] +// A dequantiser's signature is its block format: pointer, length, scale, +// output, element count. Grouping them would add single-use structs per +// format. +#![allow(clippy::too_many_arguments)] +// Doc-comment continuation indentation and list nesting, on comments that +// already read correctly. +#![allow(clippy::doc_lazy_continuation)] +#![allow(clippy::doc_overindented_list_items)] //! # jouleclaw-loader-gguf //! diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/bpe.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/bpe.rs index f2e001ec1..1df6cef0f 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/bpe.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/bpe.rs @@ -191,7 +191,7 @@ fn bpe_pre_tokenizes_on_whitespace() { tokens.push((format!("{}c", space_marker), 0.0)); // " c" tokens.push((space_marker.clone(), 0.0)); // " " alone - let owned_vocab: Vec<(String, f32)> = tokens.iter().cloned().collect(); + let owned_vocab: Vec<(String, f32)> = tokens.to_vec(); let pairs: Vec<(&str, f32)> = owned_vocab.iter() .map(|(s, sc)| (s.as_str(), *sc)).collect(); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/bpe_regex.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/bpe_regex.rs index 368c047d8..729b8b5da 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/bpe_regex.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/bpe_regex.rs @@ -121,7 +121,7 @@ fn regex_bpe_round_trips() { tokens.push((space_mark.clone(), 0.0)); // Add " w", " h" forms for "hello world" round-trip tokens.push((format!("{}w", space_mark), 0.0)); - let owned = tokens.iter().cloned().collect::>(); + let owned = tokens.to_vec(); let pairs: Vec<(&str, f32)> = owned.iter() .map(|(s, sc)| (s.as_str(), *sc)).collect(); let v = vocab_with_merges(pairs, vec![(&space_mark, "w")], None, None, None); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/dequant.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/dequant.rs index 6eb4dc1d2..2a4c640bf 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/dequant.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/dequant.rs @@ -54,7 +54,7 @@ fn q8_0_unit_scale_returns_quants_as_floats() { block[2 + i] = q as u8; } let out = dequantize_q8_0(&block, 32).unwrap(); - let expected: Vec = (0..32).map(|i| (i as i32 - 5) as f32).collect(); + let expected: Vec = (0..32).map(|i| (i - 5) as f32).collect(); assert_eq!(out, expected); } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/tokenizer.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/tokenizer.rs index a93993f7a..eaf68e54b 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/tokenizer.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/tokenizer.rs @@ -187,7 +187,7 @@ fn encode_falls_back_to_bytes_for_unknown_chars() { for b in 0..256u32 { tokens.push((format!("<0x{:02X}>", b), -100.0)); } - let owned: Vec<(String, f32)> = tokens.iter().cloned().collect(); + let owned: Vec<(String, f32)> = tokens.to_vec(); let pairs: Vec<(&str, f32)> = owned.iter() .map(|(s, sc)| (s.as_str(), *sc)).collect(); -- 2.54.0 (Apple Git-157) From fd25cb8b705a4cc930341915eb7d792e064b281f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 20:30:31 -0400 Subject: [PATCH 2/5] jouleclaw-loader-gguf: apply clippy's own suggestions, 81 -> 40 `cargo clippy --fix` only applies MachineApplicable suggestions, and most of this crate's remaining lints carry MaybeIncorrect ones. Those are still exact byte spans with exact replacement text; "maybe incorrect" means clippy will not promise the semantics, not that the text is wrong. So scripts/../apply-suggestions.py reads the JSON and applies them, and the crate's 343 tests decide. 96 spans across 26 files: collapsible_if, redundant_guards, needless_late_init, unnecessary_cast, manual_is_multiple_of, manual_repeat_n, unnecessary_sort_by, manual_slice_size_calculation, unused_mut, needless_lifetimes, needless_borrows_for_generic_args, double_parens. One suggestion class must never be applied this way, and it cost a revert to learn: `needless_range_loop` emits a *template*, not a replacement -- for (i, ) in w.iter_mut().enumerate().take(total) -- leaving both the binding name and the body's `w[i]` uses for a human. Applied literally it writes `` into the source, which does not parse; four files failed to compile. They were reverted to HEAD and the applier now refuses any replacement containing `<` and `>`. The lesson that actually matters is the ordering: the first batch of 69 was verified green and then left uncommitted while a riskier batch ran on top, so reverting the breakage could not be done without also losing verified work. Commit verified progress before the next batch. Verified: 0 errors, 343 tests pass. Co-Authored-By: Claude Opus 5 --- apply-suggestions.py | 96 +++++++++++++++++++ census.py | 22 +++++ .../examples/load_model.rs | 4 +- .../examples/load_openvino.rs | 2 +- .../examples/load_safetensors.rs | 2 +- .../jouleclaw-loader-gguf/src/attention.rs | 2 +- .../src/bpe_tokenizer.rs | 10 +- .../jouleclaw-loader-gguf/src/dequant.rs | 42 ++++---- .../crates/jouleclaw-loader-gguf/src/gemma.rs | 5 +- .../jouleclaw-loader-gguf/src/gemma4.rs | 2 +- .../jouleclaw-loader-gguf/src/granite_h.rs | 2 +- .../crates/jouleclaw-loader-gguf/src/lib.rs | 6 +- .../crates/jouleclaw-loader-gguf/src/llama.rs | 9 +- .../jouleclaw-loader-gguf/src/matmul_f.rs | 8 +- .../jouleclaw-loader-gguf/src/matmul_q4_k.rs | 14 ++- .../jouleclaw-loader-gguf/src/matmul_q5_k.rs | 14 ++- .../crates/jouleclaw-loader-gguf/src/moe.rs | 5 +- .../jouleclaw-loader-gguf/src/nemotron_h.rs | 2 +- .../jouleclaw-loader-gguf/src/qwen35moe.rs | 2 +- .../src/safetensors_reader.rs | 4 +- .../crates/jouleclaw-loader-gguf/src/ssm.rs | 2 +- .../jouleclaw-loader-gguf/src/tflite.rs | 2 +- .../jouleclaw-loader-gguf/src/tier_store.rs | 4 +- 23 files changed, 184 insertions(+), 77 deletions(-) create mode 100644 apply-suggestions.py create mode 100644 census.py diff --git a/apply-suggestions.py b/apply-suggestions.py new file mode 100644 index 000000000..fe07530a5 --- /dev/null +++ b/apply-suggestions.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python3 +"""Apply clippy's own suggested replacements for a whitelist of lints. + +`cargo clippy --fix` applies only MachineApplicable suggestions. Most of the +remaining lints here carry MaybeIncorrect ones, which are still exact spans +with exact replacement text -- they are "maybe incorrect" because clippy will +not promise semantics, not because the text is wrong. So apply them, then let +the crate's own tests decide. + + apply-suggestions.py [ ...] +""" +import json, subprocess, sys, collections, pathlib + +feats = sys.argv[1] +want = set(sys.argv[2:]) +cmd = ["cargo","clippy","--manifest-path","jouleclaw/jouleclaw-rs/Cargo.toml", + "-p","jouleclaw-loader-gguf","--all-targets","--no-deps","--keep-going", + "--message-format=json"] +if feats: + cmd += ["--features", feats] +proc = subprocess.run(cmd, capture_output=True, text=True) + +# file -> list of (byte_start, byte_end, replacement, code) +edits = collections.defaultdict(list) +seen = set() +skipped = collections.Counter() +for line in proc.stdout.splitlines(): + try: + m = json.loads(line) + except json.JSONDecodeError: + continue + if m.get("reason") != "compiler-message": + continue + d = m.get("message", {}) + if d.get("level") != "warning": + continue + code = ((d.get("code") or {}).get("code") or "").replace("clippy::", "") + if code not in want: + continue + # Collect every span in the message that carries a replacement. + def walk(msg): + for sp in msg.get("spans", []): + if sp.get("suggested_replacement") is not None: + yield sp + for ch in msg.get("children", []): + yield from walk(ch) + got = list(walk(d)) + if not got: + skipped[code] += 1 + continue + for sp in got: + repl = sp["suggested_replacement"] + # Some suggestions are templates, not replacements: + # `needless_range_loop` emits `for (i, ) in ...`, leaving the + # binding name AND the body's `w[i]` uses for a human. Applying one + # literally produces `` in the source and will not parse. + if "<" in repl and ">" in repl: + skipped[code] += 1 + continue + key = (sp["file_name"], sp["byte_start"], sp["byte_end"]) + if key in seen: + continue + seen.add(key) + edits[sp["file_name"]].append( + (sp["byte_start"], sp["byte_end"], sp["suggested_replacement"], code)) + +applied = collections.Counter() +# clippy reports paths relative to the workspace manifest dir, not the repo root. +WS = pathlib.Path("jouleclaw/jouleclaw-rs") +for fn, es in edits.items(): + p = pathlib.Path(fn) + if not p.exists(): + p = WS / fn + if not p.exists(): + print(f" MISSING {fn}") + continue + data = p.read_bytes() + # Apply from the end so earlier offsets stay valid; drop overlaps. + es.sort(key=lambda e: e[0], reverse=True) + last_start = None + for start, end, repl, code in es: + if last_start is not None and end > last_start: + skipped[code] += 1 + continue + data = data[:start] + repl.encode() + data[end:] + last_start = start + applied[code] += 1 + p.write_bytes(data) + +print(f"applied {sum(applied.values())} edits in {len(edits)} files") +for c, n in applied.most_common(): + print(f" {n:4d} {c}") +if skipped: + print("no-suggestion or overlapping, left alone:") + for c, n in skipped.most_common(): + print(f" {n:4d} {c}") diff --git a/census.py b/census.py new file mode 100644 index 000000000..e1aef0349 --- /dev/null +++ b/census.py @@ -0,0 +1,22 @@ +import json, subprocess, sys, collections +feats = sys.argv[1] if len(sys.argv)>1 else "" +cmd=["cargo","clippy","--manifest-path","jouleclaw/jouleclaw-rs/Cargo.toml", + "-p","jouleclaw-loader-gguf","--all-targets","--no-deps","--keep-going","--message-format=json"] +if feats: cmd += ["--features", feats] +pr=subprocess.run(cmd,capture_output=True,text=True) +c=collections.Counter(); e=0; seen=set() +for l in pr.stdout.splitlines(): + try: m=json.loads(l) + except Exception: continue + if m.get("reason")!="compiler-message": continue + d=m.get("message",{}); lv=d.get("level") + if lv not in ("warning","error"): continue + code=(d.get("code") or {}).get("code") or "(uncoded)" + sp=next((s for s in d.get("spans",[]) if s.get("is_primary")),None) + k=(code, sp.get("file_name") if sp else None, sp.get("line_start") if sp else None) + if k in seen: continue + seen.add(k) + if lv=="error": e+=1; continue + c[code]+=1 +print(f"features={feats or 'default'} errors={e} warnings={sum(c.values())}") +for k,v in c.most_common(): print(f" {v:4d} {k}") diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_model.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_model.rs index 3ad301ee7..bed9da561 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_model.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_model.rs @@ -187,7 +187,7 @@ fn compute_metas(model: &GgufModel) -> Vec { fn print_top_n_by_size(metas: &[StoreTensorMeta], n: usize) { println!("=== top tensors by byte span ==="); let mut sorted: Vec<&StoreTensorMeta> = metas.iter().collect(); - sorted.sort_by(|a, b| b.size_bytes.cmp(&a.size_bytes)); + sorted.sort_by_key(|a| std::cmp::Reverse(a.size_bytes)); for m in sorted.iter().take(n) { println!( " {:<60} {:>12} bytes ({})", @@ -229,7 +229,7 @@ fn run_tier_store( store.register_tensors("m1", metas); // Pin the top-K by byte span Hot. let mut sorted: Vec<&StoreTensorMeta> = metas.iter().collect(); - sorted.sort_by(|a, b| b.size_bytes.cmp(&a.size_bytes)); + sorted.sort_by_key(|a| std::cmp::Reverse(a.size_bytes)); let t0 = Instant::now(); for (i, m) in sorted.iter().take(pin_top).enumerate() { store.pin_hot("m1", m, i as u64 + 1)?; diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_openvino.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_openvino.rs index a83b1ca5f..a75e5824a 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_openvino.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_openvino.rs @@ -55,7 +55,7 @@ fn main() -> Result<(), Box> { // Top 10 tensors. let mut sorted: Vec<&_> = m.tensors.iter().collect(); - sorted.sort_by(|a, b| b.bin_size.cmp(&a.bin_size)); + sorted.sort_by_key(|a| std::cmp::Reverse(a.bin_size)); println!(); println!("=== top 10 tensors by byte size ==="); for t in sorted.iter().take(10) { diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_safetensors.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_safetensors.rs index 9af45a78d..688863111 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_safetensors.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/load_safetensors.rs @@ -51,7 +51,7 @@ fn main() -> Result<(), Box> { // Top tensors by size. let mut sorted: Vec<&_> = m.tensors.iter().collect(); - sorted.sort_by(|a, b| b.data_len.cmp(&a.data_len)); + sorted.sort_by_key(|a| std::cmp::Reverse(a.data_len)); println!(); println!("=== top 10 tensors by byte size ==="); for t in sorted.iter().take(10) { diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/attention.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/attention.rs index e112b04af..a6065b123 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/attention.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/attention.rs @@ -278,7 +278,7 @@ fn as_u8(s: &[f32]) -> &[u8] { unsafe { std::slice::from_raw_parts( s.as_ptr() as *const u8, - s.len() * std::mem::size_of::(), + std::mem::size_of_val(s), ) } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/bpe_tokenizer.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/bpe_tokenizer.rs index 7b6b7a45d..3d19c45a2 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/bpe_tokenizer.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/bpe_tokenizer.rs @@ -337,12 +337,11 @@ impl BpeTokenizer { .map(|b| self.b2c[b as usize]) .collect(); // ignore_merges: whole pretoken already a vocab entry. - if self.ignore_merges { - if let Some(&id) = self.vocab.get(&mapped) { + if self.ignore_merges + && let Some(&id) = self.vocab.get(&mapped) { out.push(id); return; } - } // BPE merge loop. let mut parts: Vec = mapped.chars().map(String::from).collect(); loop { @@ -350,11 +349,10 @@ impl BpeTokenizer { for i in 0..parts.len().saturating_sub(1) { if let Some(&rank) = self.merges.get( &(parts[i].clone(), parts[i + 1].clone()), - ) { - if best.is_none() || rank < best.unwrap().0 { + ) + && (best.is_none() || rank < best.unwrap().0) { best = Some((rank, i)); } - } } let Some((_, i)) = best else { break }; let merged = format!("{}{}", parts[i], parts[i + 1]); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/dequant.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/dequant.rs index 14058d5bb..c70374af8 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/dequant.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/dequant.rs @@ -1110,11 +1110,11 @@ mod iq4_nl_tests { let exp = ((bits >> 23) & 0xFF) as i32 - 127 + 15; let mant = bits & 0x007F_FFFF; if exp <= 0 { - (sign << 15) as u16 + (sign << 15) } else if exp >= 31 { - ((sign << 15) | (0x1F << 10)) as u16 + ((sign << 15) | (0x1F << 10)) } else { - ((sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16)) as u16 + ((sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16)) } } @@ -1191,11 +1191,11 @@ mod q40_q41_q50_q51_q81_tests { let exp = ((bits >> 23) & 0xFF) as i32 - 127 + 15; let mant = bits & 0x007F_FFFF; if exp <= 0 { - (sign << 15) as u16 + (sign << 15) } else if exp >= 31 { - ((sign << 15) | (0x1F << 10)) as u16 + ((sign << 15) | (0x1F << 10)) } else { - ((sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16)) as u16 + ((sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16)) } } @@ -1513,9 +1513,9 @@ mod stq1_0_tests { fn f16_le(v: f32) -> [u8; 2] { let bits: u16 = match v { - x if x == 1.0 => 0x3C00, - x if x == 0.5 => 0x3800, - x if x == 2.0 => 0x4000, + 1.0 => 0x3C00, + 0.5 => 0x3800, + 2.0 => 0x4000, _ => unreachable!(), }; bits.to_le_bytes() @@ -1654,9 +1654,9 @@ mod q1_0_tests { fn f16_le(v: f32) -> [u8; 2] { let bits: u16 = match v { - x if x == 1.0 => 0x3C00, - x if x == 0.5 => 0x3800, - x if x == 2.0 => 0x4000, + 1.0 => 0x3C00, + 0.5 => 0x3800, + 2.0 => 0x4000, _ => unreachable!(), }; bits.to_le_bytes() @@ -1669,7 +1669,7 @@ mod q1_0_tests { let mut buf = Vec::new(); buf.extend_from_slice(&f16_le(0.5)); buf.push(0xA5); - buf.extend(std::iter::repeat(0u8).take(15)); // rest = bit 0 → -0.5 + buf.extend(std::iter::repeat_n(0u8, 15)); // rest = bit 0 → -0.5 let v = dequantize_q1_0(&buf, Q1_0_BLOCK_ELEMS).unwrap(); assert_eq!(&v[0..8], &[0.5, -0.5, 0.5, -0.5, -0.5, 0.5, -0.5, 0.5]); assert!(v[8..].iter().all(|&x| x == -0.5)); @@ -1681,10 +1681,10 @@ mod q1_0_tests { let mut buf = Vec::new(); // block0: d=1.0, all bits 1 → all +1.0 buf.extend_from_slice(&f16_le(1.0)); - buf.extend(std::iter::repeat(0xFFu8).take(16)); + buf.extend(std::iter::repeat_n(0xFFu8, 16)); // block1: d=2.0, all bits 0 → all -2.0 buf.extend_from_slice(&f16_le(2.0)); - buf.extend(std::iter::repeat(0x00u8).take(16)); + buf.extend(std::iter::repeat_n(0x00u8, 16)); let v = dequantize_q1_0(&buf, 2 * Q1_0_BLOCK_ELEMS).unwrap(); assert!(v[..128].iter().all(|&x| x == 1.0)); assert!(v[128..].iter().all(|&x| x == -2.0)); @@ -1765,9 +1765,9 @@ mod q2_0_tests { // minimal round-trippable encodings for the small exact values // used in tests (1.0, 0.5, 2.0) — exercises f16_from_le too. let bits: u16 = match v { - x if x == 1.0 => 0x3C00, - x if x == 0.5 => 0x3800, - x if x == 2.0 => 0x4000, + 1.0 => 0x3C00, + 0.5 => 0x3800, + 2.0 => 0x4000, _ => unreachable!("test uses only 1.0/0.5/2.0"), }; bits.to_le_bytes() @@ -1782,7 +1782,7 @@ mod q2_0_tests { let mut buf = Vec::new(); buf.extend_from_slice(&f16_le(0.5)); buf.push(0xE4); - buf.extend(std::iter::repeat(0u8).take(31)); // rest of qs (code 0 → -1) + buf.extend(std::iter::repeat_n(0u8, 31)); // rest of qs (code 0 → -1) let v = dequantize_q2_0(&buf, Q2_0_BLOCK_ELEMS).unwrap(); assert_eq!(&v[0..4], &[-0.5, 0.0, 0.5, 1.0]); // every remaining element is code 0 → (0-1)*0.5 = -0.5 @@ -1796,10 +1796,10 @@ mod q2_0_tests { let mut buf = Vec::new(); // block 0: d=1.0, all code 2 (+1) → all +1.0 buf.extend_from_slice(&f16_le(1.0)); - buf.extend(std::iter::repeat(0xAAu8).take(32)); // 0b10_10_10_10 → all q=2 + buf.extend(std::iter::repeat_n(0xAAu8, 32)); // 0b10_10_10_10 → all q=2 // block 1: d=2.0, all code 0 (-1) → all -2.0 buf.extend_from_slice(&f16_le(2.0)); - buf.extend(std::iter::repeat(0x00u8).take(32)); + buf.extend(std::iter::repeat_n(0x00u8, 32)); let v = dequantize_q2_0(&buf, 2 * Q2_0_BLOCK_ELEMS).unwrap(); assert!(v[..128].iter().all(|&x| x == 1.0)); assert!(v[128..].iter().all(|&x| x == -2.0)); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/gemma.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/gemma.rs index c25487dd6..73e86eac9 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/gemma.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/gemma.rs @@ -438,11 +438,10 @@ mod tests { match remap_gemma_tensor_name(name) { Some(g) => { text_mapped += 1; - if let Some(rest) = g.strip_prefix("blk.") { - if let Some(d) = rest.find('.') { + if let Some(rest) = g.strip_prefix("blk.") + && let Some(d) = rest.find('.') { seen_layers.insert(rest[..d].parse::().unwrap()); } - } if g == "per_layer_token_embd.weight" { have_ple = true; } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/gemma4.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/gemma4.rs index 6d7b9ca61..ccc92deaf 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/gemma4.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/gemma4.rs @@ -148,7 +148,7 @@ pub struct ForwardOut { pub(crate) fn gelu_tanh(x: f32) -> f32 { // gelu_pytorch_tanh const C: f32 = 0.797_884_6; // sqrt(2/pi) - 0.5 * x * (1.0 + ((C * (x + 0.044715 * x * x * x))).tanh()) + 0.5 * x * (1.0 + (C * (x + 0.044715 * x * x * x)).tanh()) } /// `y[o] = Σ_i x[i] * W[o,i]` for the decode hot path (seq=1). diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/granite_h.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/granite_h.rs index 420d3f848..73718d901 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/granite_h.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/granite_h.rs @@ -1036,7 +1036,7 @@ pub(crate) mod tests { let vocab = cfg.vocab_size as u64; let mut t: Vec<(String, Vec, Vec)> = Vec::new(); let mut seed = 1usize; - let mut push = |name: String, shape: Vec, t: &mut Vec<_>, seed: &mut usize| { + let push = |name: String, shape: Vec, t: &mut Vec<_>, seed: &mut usize| { let n: usize = shape.iter().product::() as usize; t.push((name, shape, synth(n, *seed))); *seed += 1; diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs index 9eae89ef9..21b28a213 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs @@ -1013,7 +1013,7 @@ fn apply_prefetch_policy( ranked[w].2 = next_off.saturating_sub(ranked[w].1); } // Sort by computed byte span descending; take top N. - ranked.sort_by(|a, b| b.2.cmp(&a.2)); + ranked.sort_by_key(|a| std::cmp::Reverse(a.2)); for &(_, off, span) in ranked.iter().take(n) { let start = data_offset + off as usize; let end = (start + span as usize).min(bytes.len()); @@ -1341,8 +1341,8 @@ pub fn qmatmul_expert( /// non-quantised tensors must be dequantised to f32 instead. pub fn qmatmul_ok(dtype: GgmlType, in_d: usize) -> bool { match dtype { - GgmlType::Q8_0 => in_d % 32 == 0, - GgmlType::Q4_K | GgmlType::Q5_K | GgmlType::Q6_K => in_d % 256 == 0, + GgmlType::Q8_0 => in_d.is_multiple_of(32), + GgmlType::Q4_K | GgmlType::Q5_K | GgmlType::Q6_K => in_d.is_multiple_of(256), _ => false, } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/llama.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/llama.rs index a829b4181..068ffe7a1 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/llama.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/llama.rs @@ -544,21 +544,18 @@ fn build_block( // intermediates instead of three separate per-projection command // buffers (each a commit+wait). Value-preserving; decode only // (seq_len == 1). Prefill / other quant keep the unfused path. - if seq_len == 1 && std::env::var("NO_MLP_BLOCK").is_err() { - if let ( + if seq_len == 1 && std::env::var("NO_MLP_BLOCK").is_err() + && let ( &Weight::Q4K { node: gn, out: gf, k: gh }, &Weight::Q4K { node: un, out: uf, k: uh }, &Weight::Q6K { node: dn, out: dout, k: dk }, ) = (&w_gate, &w_up, &w_down) - { - if uh == gh && uf == gf && dk == gf && dout == gh + && uh == gh && uf == gf && dk == gf && dout == gh && gh % 256 == 0 && gf % 256 == 0 { let ffn_out = g.mlp_block_q4k_q6k(xn2, gn, un, dn, gh, gf); return Ok(g.add(after_attn, ffn_out)); } - } - } let gate = wmm(g, xn2, &w_gate); let gate = g.activation(gate, ActivationKind::SiLU); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs index a4afd92bb..ea1cd28c4 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs @@ -236,7 +236,7 @@ fn bytemuck_f32_as_u8(s: &[f32]) -> &[u8] { unsafe { std::slice::from_raw_parts( s.as_ptr() as *const u8, - s.len() * std::mem::size_of::(), + std::mem::size_of_val(s), ) } } @@ -346,11 +346,11 @@ mod tests { let exp = ((bits >> 23) & 0xFF) as i32 - 127 + 15; let mant = bits & 0x007F_FFFF; if exp <= 0 { - (sign << 15) as u16 + (sign << 15) } else if exp >= 31 { - ((sign << 15) | (0x1F << 10)) as u16 + ((sign << 15) | (0x1F << 10)) } else { - ((sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16)) as u16 + ((sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16)) } } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_q4_k.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_q4_k.rs index 78f93fea3..f497913c2 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_q4_k.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_q4_k.rs @@ -287,17 +287,15 @@ unsafe fn sub_block_neon_sums( // half (low or high) gives 32 i8 values. let qs_a = vld1q_u8(qs_ptr); let qs_b = vld1q_u8(qs_ptr.add(16)); - let nibbles_a; - let nibbles_b; - if high_nibble { + + + let (nibbles_a, nibbles_b) = if high_nibble { // Take high 4 bits. - nibbles_a = vshrq_n_u8(qs_a, 4); - nibbles_b = vshrq_n_u8(qs_b, 4); + (vshrq_n_u8(qs_a, 4), vshrq_n_u8(qs_b, 4)) } else { let mask = vdupq_n_u8(0x0F); - nibbles_a = vandq_u8(qs_a, mask); - nibbles_b = vandq_u8(qs_b, mask); - } + (vandq_u8(qs_a, mask), vandq_u8(qs_b, mask)) + }; // Widen u8 → u16 (zero-extend). let u16_a_lo = vmovl_u8(vget_low_u8(nibbles_a)); let u16_a_hi = vmovl_u8(vget_high_u8(nibbles_a)); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_q5_k.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_q5_k.rs index 7bd4b504f..561846447 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_q5_k.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_q5_k.rs @@ -227,16 +227,14 @@ unsafe fn q5_k_sub_block_neon_sums( let high_half = sub % 2 == 1; let qs_a = vld1q_u8(qs_ptr); let qs_b = vld1q_u8(qs_ptr.add(16)); - let nib_a; - let nib_b; - if high_half { - nib_a = vshrq_n_u8(qs_a, 4); - nib_b = vshrq_n_u8(qs_b, 4); + + + let (nib_a, nib_b) = if high_half { + (vshrq_n_u8(qs_a, 4), vshrq_n_u8(qs_b, 4)) } else { let mask = vdupq_n_u8(0x0F); - nib_a = vandq_u8(qs_a, mask); - nib_b = vandq_u8(qs_b, mask); - } + (vandq_u8(qs_a, mask), vandq_u8(qs_b, mask)) + }; // Extract the sub-th bit from each qh byte; shift it // into position 4 so we can OR into the nibble. // For output element k = sub*32 + l (l in 0..32): high diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/moe.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/moe.rs index e280e5c36..ab687f174 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/moe.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/moe.rs @@ -245,15 +245,14 @@ pub fn advise_stacked_experts( if active.contains(&e) { continue; } - if let Some((start, end)) = tensor.expert_byte_range(e) { - if end <= data_len { + if let Some((start, end)) = tensor.expert_byte_range(e) + && end <= data_len { let _ = advise( &data[start as usize..end as usize], AdviceKind::DontNeed, ); count += 1; } - } } Ok(count) } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/nemotron_h.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/nemotron_h.rs index a71200d11..32be7847c 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/nemotron_h.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/nemotron_h.rs @@ -822,7 +822,7 @@ pub(crate) mod tests { let vocab = cfg.vocab_size; let mut t: Vec<(String, Vec, Vec)> = Vec::new(); let mut seed = 1; - let mut push = |name: String, shape: Vec, t: &mut Vec<_>, seed: &mut usize| { + let push = |name: String, shape: Vec, t: &mut Vec<_>, seed: &mut usize| { let n: usize = shape.iter().product::() as usize; t.push((name, shape, synth(n, *seed))); *seed += 1; diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/qwen35moe.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/qwen35moe.rs index c241f6b88..202867da1 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/qwen35moe.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/qwen35moe.rs @@ -105,7 +105,7 @@ impl QwenConfig { }) } fn is_recurrent(&self, l: usize) -> bool { - (l + 1) % self.full_attn_interval != 0 + !(l + 1).is_multiple_of(self.full_attn_interval) } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/safetensors_reader.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/safetensors_reader.rs index f3a6113ee..f9834d263 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/safetensors_reader.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/safetensors_reader.rs @@ -581,7 +581,7 @@ mod tests { #[test] fn read_rejects_file_too_small() { let path = tempfile("tiny"); - std::fs::write(&path, &[0u8, 1, 2]).unwrap(); + std::fs::write(&path, [0u8, 1, 2]).unwrap(); let r = read_safetensors_file(&path); assert!(matches!(r, Err(SafetensorsError::FileTooSmall(3)))); let _ = std::fs::remove_file(path); @@ -591,7 +591,7 @@ mod tests { fn read_rejects_oversized_header() { let path = tempfile("bad-header"); // header_len = u64::MAX - std::fs::write(&path, &(u64::MAX).to_le_bytes()).unwrap(); + std::fs::write(&path, (u64::MAX).to_le_bytes()).unwrap(); let r = read_safetensors_file(&path); assert!(matches!(r, Err(SafetensorsError::HeaderTooLarge { .. }))); let _ = std::fs::remove_file(path); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/ssm.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/ssm.rs index 99ca33dd2..3a243a5ff 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/ssm.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/ssm.rs @@ -366,7 +366,7 @@ pub fn gated_rmsnorm_inplace( assert_eq!(y.len(), z.len()); assert_eq!(y.len(), weight.len()); let n = y.len(); - assert!(group_size > 0 && n % group_size == 0, + assert!(group_size > 0 && n.is_multiple_of(group_size), "len {n} not divisible by group_size {group_size}"); for i in 0..n { y[i] *= silu(z[i]); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/tflite.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/tflite.rs index a28a3256c..8539d5b07 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/tflite.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/tflite.rs @@ -554,7 +554,7 @@ mod tests { // Helper: push a value, pad-aligned to its width. fn pad_to(buf: &mut Vec, align: usize) { - while buf.len() % align != 0 { + while !buf.len().is_multiple_of(align) { buf.push(0); } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/tier_store.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/tier_store.rs index 34ed8d008..f1dd6ab05 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/tier_store.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/tier_store.rs @@ -573,8 +573,8 @@ impl TierStore { /// the lifetime of the borrow; promotion / demotion calls /// don't invalidate the underlying mmap, only the LRU /// metadata. - pub fn bytes<'a, R>( - &'a self, + pub fn bytes( + &self, model_id: &str, meta: &StoreTensorMeta, f: impl FnOnce(&[u8]) -> R, -- 2.54.0 (Apple Git-157) From fb295fb2813973997e60a461880effc83b5acd26 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 20:31:17 -0400 Subject: [PATCH 3/5] jouleclaw-loader-gguf: 40 -> 14 clippy warnings 26 more of clippy's own suggestions: unused_parens (18 of them introduced by the previous batch's rewrites), collapsible_match, derivable_impls, manual_range_contains, unnecessary_mut_passed. 0 errors, 343 tests pass. Co-Authored-By: Claude Opus 5 --- .../crates/jouleclaw-loader-gguf/src/dequant.rs | 14 +++++++------- .../crates/jouleclaw-loader-gguf/src/hybrid_lm.rs | 2 +- .../crates/jouleclaw-loader-gguf/src/lib.rs | 7 ++----- .../crates/jouleclaw-loader-gguf/src/matmul_f.rs | 6 +++--- .../crates/jouleclaw-loader-gguf/src/openvino.rs | 5 ++--- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/dequant.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/dequant.rs index c70374af8..194b1c1a7 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/dequant.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/dequant.rs @@ -905,7 +905,7 @@ pub fn mxfp4_e8m0_to_scale(byte: u8) -> f32 { return 0.0; } let exp = byte as i32 - 127; - if exp >= -126 && exp <= 127 { + if (-126..=127).contains(&exp) { let bits = ((exp + 127) as u32) << 23; f32::from_bits(bits) } else if exp < -126 { @@ -1110,11 +1110,11 @@ mod iq4_nl_tests { let exp = ((bits >> 23) & 0xFF) as i32 - 127 + 15; let mant = bits & 0x007F_FFFF; if exp <= 0 { - (sign << 15) + sign << 15 } else if exp >= 31 { - ((sign << 15) | (0x1F << 10)) + (sign << 15) | (0x1F << 10) } else { - ((sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16)) + (sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16) } } @@ -1191,11 +1191,11 @@ mod q40_q41_q50_q51_q81_tests { let exp = ((bits >> 23) & 0xFF) as i32 - 127 + 15; let mant = bits & 0x007F_FFFF; if exp <= 0 { - (sign << 15) + sign << 15 } else if exp >= 31 { - ((sign << 15) | (0x1F << 10)) + (sign << 15) | (0x1F << 10) } else { - ((sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16)) + (sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16) } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/hybrid_lm.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/hybrid_lm.rs index c6fae6788..471cb3e19 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/hybrid_lm.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/hybrid_lm.rs @@ -119,7 +119,7 @@ pub fn generate(model: &M, prompt: &[u32], params: &GenerateParams) let mut out = Vec::new(); let mut logits = model.forward_prompt(prompt, &mut state); for _ in 0..params.max_new_tokens { - let token = sample_logits_with_history(&logits, &mut sampling, &history); + let token = sample_logits_with_history(&logits, &sampling, &history); if params.stop_tokens.contains(&token) { break; } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs index 21b28a213..48ec3ae3d 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/lib.rs @@ -685,6 +685,7 @@ pub fn read_gguf_file_with_policy>( /// policy mismatch costs at most one extra page-fault round /// vs the no-prefetch path. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Default)] pub enum PrefetchPolicy { /// No prefetch — pages fault in synchronously on first /// touch. Identical to pre-wave-2 behaviour. Pick this @@ -697,6 +698,7 @@ pub enum PrefetchPolicy { /// embeddings, Sequential for matmul weights, WillNeed /// for norms). Default. Best cold-cache TTFT for /// full-model inference. + #[default] All, /// Hint the top `N` tensors by byte span as `WillNeed`, /// leaving the long tail to fault in lazily. Per-tensor @@ -713,11 +715,6 @@ pub enum PrefetchPolicy { PerTensorOnly, } -impl Default for PrefetchPolicy { - fn default() -> Self { - Self::All - } -} /// Parse a GGUF model whose tensor data is already mmapped. Header /// is parsed via a `Cursor` over the mmap; the tensor data section diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs index ea1cd28c4..359706545 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs @@ -346,11 +346,11 @@ mod tests { let exp = ((bits >> 23) & 0xFF) as i32 - 127 + 15; let mant = bits & 0x007F_FFFF; if exp <= 0 { - (sign << 15) + sign << 15 } else if exp >= 31 { - ((sign << 15) | (0x1F << 10)) + (sign << 15) | (0x1F << 10) } else { - ((sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16)) + (sign << 15) | ((exp as u16) << 10) | ((mant >> 13) as u16) } } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/openvino.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/openvino.rs index ed7462b8e..7bf62ddf8 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/openvino.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/openvino.rs @@ -360,8 +360,8 @@ pub fn parse_openvino_xml( _ => {} } } - Event::End(e) => { - if e.name().as_ref() == b"layer" && in_layer_const { + Event::End(e) + if e.name().as_ref() == b"layer" && in_layer_const => { tensors.push(OvTensorInfo { id: cur_id, name: std::mem::take(&mut cur_name), @@ -372,7 +372,6 @@ pub fn parse_openvino_xml( }); in_layer_const = false; } - } _ => {} } buf.clear(); -- 2.54.0 (Apple Git-157) From 2f71fde057a1b1aedb52f5b9f6ca39688d17a962 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 20:32:41 -0400 Subject: [PATCH 4/5] jouleclaw-loader-gguf: 14 -> 12, the chunks_exact rewrites 16 `chunks_exact(N)` -> `as_chunks::().0` across 8 files, which is the batch that needed the tests rather than the compiler: the two forms differ in how they expose the trailing remainder, and these walk quantised block layouts. The first guard against template suggestions was too crude and rejected all 28 of these, because `as_chunks::<2>()` legitimately contains angle brackets. It now rejects only a bare lowercase word in brackets that no `::` precedes, which is what a placeholder looks like. One follow-on the suggestion cannot express: `as_chunks` yields `&[u8; N]`, so the `c.try_into().unwrap()` that `chunks_exact(8)` needed in onnx.rs became ill-typed. Dropped to `*c`. Verified: 0 errors, 343 tests pass. Co-Authored-By: Claude Opus 5 --- apply-suggestions.py | 8 ++++++-- .../crates/jouleclaw-loader-gguf/src/cohere2_moe.rs | 4 ++-- .../crates/jouleclaw-loader-gguf/src/falcon_h1.rs | 4 ++-- .../crates/jouleclaw-loader-gguf/src/granite_h.rs | 8 ++++---- .../crates/jouleclaw-loader-gguf/src/matmul_bf16.rs | 2 +- .../crates/jouleclaw-loader-gguf/src/matmul_f.rs | 2 +- .../crates/jouleclaw-loader-gguf/src/mellum.rs | 4 ++-- .../crates/jouleclaw-loader-gguf/src/nemotron_h.rs | 4 ++-- .../crates/jouleclaw-loader-gguf/src/onnx.rs | 10 +++++----- 9 files changed, 25 insertions(+), 21 deletions(-) diff --git a/apply-suggestions.py b/apply-suggestions.py index fe07530a5..536b4a8bc 100644 --- a/apply-suggestions.py +++ b/apply-suggestions.py @@ -9,7 +9,7 @@ the crate's own tests decide. apply-suggestions.py [ ...] """ -import json, subprocess, sys, collections, pathlib +import json, re, subprocess, sys, collections, pathlib feats = sys.argv[1] want = set(sys.argv[2:]) @@ -54,7 +54,11 @@ for line in proc.stdout.splitlines(): # `needless_range_loop` emits `for (i, ) in ...`, leaving the # binding name AND the body's `w[i]` uses for a human. Applying one # literally produces `` in the source and will not parse. - if "<" in repl and ">" in repl: + # A template placeholder is a bare lowercase word in angle brackets, + # like ``. A turbofish is not: `as_chunks::<2>()` is a real + # suggestion and the first version of this guard rejected all 28 of + # them. Reject only an angle-bracketed word that no `::` precedes. + if re.search(r"(?", repl): skipped[code] += 1 continue key = (sp["file_name"], sp["byte_start"], sp["byte_end"]) diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/cohere2_moe.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/cohere2_moe.rs index ecb9f3b9e..bdadbc156 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/cohere2_moe.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/cohere2_moe.rs @@ -216,7 +216,7 @@ impl Cohere2MoeModel { } let bytes = shards[m.shard].tensor_bytes(t); Ok(bytes - .chunks_exact(2) + .as_chunks::<2>().0.iter() .map(|c| bf16_to_f32(u16::from_le_bytes([c[0], c[1]]))) .collect()) }; @@ -293,7 +293,7 @@ impl Cohere2MoeModel { let k = t.shape[1] as usize; let bytes = self.shards[self.embeddings.shard].tensor_bytes(t); let row = &bytes[(token as usize) * k * 2..((token as usize) + 1) * k * 2]; - for (i, c) in row.chunks_exact(2).enumerate() { + for (i, c) in row.as_chunks::<2>().0.iter().enumerate() { out[i] = bf16_to_f32(u16::from_le_bytes([c[0], c[1]])); } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/falcon_h1.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/falcon_h1.rs index 05b1ba5d7..ed391d2f6 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/falcon_h1.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/falcon_h1.rs @@ -182,7 +182,7 @@ impl FalconH1Model { return Err(FalconH1Error::WrongDtype(name.to_string(), t.dtype)); } let bytes = st.tensor_bytes(t); - Ok(bytes.chunks_exact(2) + Ok(bytes.as_chunks::<2>().0.iter() .map(|c| bf16_to_f32(u16::from_le_bytes([c[0], c[1]]))) .collect()) }; @@ -274,7 +274,7 @@ impl FalconH1Model { let bytes = self.st.tensor_bytes(t); let row = &bytes[(token as usize) * k * 2..((token as usize) + 1) * k * 2]; let mult = self.config.embedding_multiplier; - for (i, c) in row.chunks_exact(2).enumerate() { + for (i, c) in row.as_chunks::<2>().0.iter().enumerate() { out[i] = bf16_to_f32(u16::from_le_bytes([c[0], c[1]])) * mult; } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/granite_h.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/granite_h.rs index 73718d901..30f6f7ada 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/granite_h.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/granite_h.rs @@ -238,7 +238,7 @@ impl GraniteHModel { if t.dtype != SafetensorsDtype::BF16 { return Err(GraniteHError::WrongDtype(name.to_string(), t.dtype)); } - Ok(st.tensor_bytes(t).chunks_exact(2) + Ok(st.tensor_bytes(t).as_chunks::<2>().0.iter() .map(|c| bf16_to_f32(u16::from_le_bytes([c[0], c[1]]))) .collect()) }; @@ -399,7 +399,7 @@ impl GraniteHModel { let k = t.shape[1] as usize; let bytes = self.tensor_bytes(self.embeddings); let row = &bytes[(token as usize) * k * 2..((token as usize) + 1) * k * 2]; - for (i, c) in row.chunks_exact(2).enumerate() { + for (i, c) in row.as_chunks::<2>().0.iter().enumerate() { out[i] = bf16_to_f32(u16::from_le_bytes([c[0], c[1]])); } } @@ -1202,7 +1202,7 @@ pub(crate) mod tests { build_tiny_checkpoint(&path, &tiny_config()); let st = read_safetensors_file(&path).unwrap(); for info in &st.tensors { - let vals: Vec = st.tensor_bytes(info).chunks_exact(2) + let vals: Vec = st.tensor_bytes(info).as_chunks::<2>().0.iter() .map(|c| crate::matmul_bf16::bf16_to_f32( u16::from_le_bytes([c[0], c[1]]))) .collect(); @@ -1262,7 +1262,7 @@ pub(crate) mod tests { build_tiny_checkpoint(&p2, &tiny_config()); let st = read_safetensors_file(&p2).unwrap(); for info in &st.tensors { - let vals: Vec = st.tensor_bytes(info).chunks_exact(2) + let vals: Vec = st.tensor_bytes(info).as_chunks::<2>().0.iter() .map(|c| crate::matmul_bf16::bf16_to_f32( u16::from_le_bytes([c[0], c[1]]))) .collect(); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_bf16.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_bf16.rs index d6d4d3953..5fb483828 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_bf16.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_bf16.rs @@ -420,7 +420,7 @@ mod tests { m: usize, n: usize, k: usize, ) -> Vec { let w_f32: Vec = w_bf16 - .chunks_exact(2) + .as_chunks::<2>().0.iter() .map(|c| bf16_to_f32(u16::from_le_bytes([c[0], c[1]]))) .collect(); let mut out = vec![0f32; m * n]; diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs index 359706545..42e9986b1 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/matmul_f.rs @@ -328,7 +328,7 @@ mod tests { matmul_f16_scalar(&a, &w_f16, &bias, m, n, k, &mut got); // Reference via the f16-decoded-as-f32 weight. let w_f32_via_f16: Vec = w_f16 - .chunks_exact(2) + .as_chunks::<2>().0.iter() .map(|c| f16_to_f32(u16::from_le_bytes([c[0], c[1]]))) .collect(); let expected = naive_matmul_f32(&a, &w_f32_via_f16, &bias, m, n, k); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/mellum.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/mellum.rs index 58028fb0f..3debb8f3a 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/mellum.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/mellum.rs @@ -286,7 +286,7 @@ impl MellumModel { } let bytes = shards[m.shard].tensor_bytes(t); Ok(bytes - .chunks_exact(2) + .as_chunks::<2>().0.iter() .map(|c| bf16_to_f32(u16::from_le_bytes([c[0], c[1]]))) .collect()) }; @@ -381,7 +381,7 @@ impl MellumModel { let k = t.shape[1] as usize; let bytes = self.shards[self.embeddings.shard].tensor_bytes(t); let row = &bytes[(token as usize) * k * 2..((token as usize) + 1) * k * 2]; - for (i, c) in row.chunks_exact(2).enumerate() { + for (i, c) in row.as_chunks::<2>().0.iter().enumerate() { out[i] = bf16_to_f32(u16::from_le_bytes([c[0], c[1]])); } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/nemotron_h.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/nemotron_h.rs index 32be7847c..65f654bef 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/nemotron_h.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/nemotron_h.rs @@ -194,7 +194,7 @@ impl NemotronHModel { return Err(NemotronHError::WrongDtype(name.to_string(), t.dtype)); } let bytes = st.tensor_bytes(t); - Ok(bytes.chunks_exact(2) + Ok(bytes.as_chunks::<2>().0.iter() .map(|c| bf16_to_f32(u16::from_le_bytes([c[0], c[1]]))) .collect()) }; @@ -288,7 +288,7 @@ impl NemotronHModel { let k = t.shape[1] as usize; let bytes = self.st.tensor_bytes(t); let row = &bytes[(token as usize) * k * 2..((token as usize) + 1) * k * 2]; - for (i, c) in row.chunks_exact(2).enumerate() { + for (i, c) in row.as_chunks::<2>().0.iter().enumerate() { out[i] = bf16_to_f32(u16::from_le_bytes([c[0], c[1]])); } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/onnx.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/onnx.rs index 70172a435..6664a4354 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/onnx.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/onnx.rs @@ -376,7 +376,7 @@ fn parse_tensor( (4, 2) => { let (s, e) = p.read_length_delimited_range()?; let chunk = &mmap[s..e]; - for c in chunk.chunks_exact(4) { + for c in chunk.as_chunks::<4>().0 { typed.float_data.push(f32::from_le_bytes([c[0], c[1], c[2], c[3]])); } } @@ -408,10 +408,10 @@ fn parse_tensor( (10, 2) => { let (s, e) = p.read_length_delimited_range()?; let chunk = &mmap[s..e]; - for c in chunk.chunks_exact(8) { - typed.double_data.push(f64::from_le_bytes( - c.try_into().unwrap(), - )); + // `as_chunks` already yields `&[u8; 8]`, so the `try_into` + // the old `chunks_exact(8)` needed is gone. + for c in chunk.as_chunks::<8>().0 { + typed.double_data.push(f64::from_le_bytes(*c)); } } // TensorProto.uint64_data = 11 (uint64 repeated, packed varint) -- 2.54.0 (Apple Git-157) From f5760cc22d0ca8c3fa99a5ca4b0e04c392d526f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 20:40:32 -0400 Subject: [PATCH 5/5] jouleclaw-loader-gguf: 0 warnings, and -D warnings now gates it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 12 -> 0, in both the default and `rayon fabric-meter` configurations, and the crate joins the `-D warnings` clippy step in standards-build.yml. The last dozen needed judgement rather than a script: - two `from_str` methods that are not FromStr-shaped (one returns Option, the other is infallible) renamed to `parse`. Both are file-local, so there is no caller churn -- except that my check for callers missed mlx.rs, which reaches SafetensorsDtype through a type ALIAS, so `SafetensorsDtype::from_str(` never matched. Renaming wants a grep for the method name across the workspace, not for the type names you happen to know. - `assert!(FLASH_KV_CHUNK.is_power_of_two())` and `>= 64` moved out of a `#[test]` into `const _: () = assert!(..)` beside the constant. These are properties of a constant; a test only holds them if someone runs it. - the `kv_quant_tests` module moved to the end of kv_cache_inplace.rs, because 300 lines of real items sat after it. - an example discarded the `Result` from `matmul_bf16_threaded`, so a failure would have been timed as a success. - `needless_range_loop` allowed per-file in three benches and one test: those are separate crates, so src/lib.rs's decision does not reach them. - checked_div for two guarded divisions, and one unused `mut`. Two near-misses worth recording: - `fabric_dispatch.rs` calls `ledger_record` twice with identical arguments, which reads exactly like a copy-paste bug. The comment three lines above says attention is "recorded as two GEMM-equivalents" so the measured µJ/GFLOP table prices it. Deliberate. Left alone. - clippy's `chunks_exact_mut` suggestion is `as_chunks_mut::().0.iter()` -- a non-mutable iterator over a mutable chunk. It does not compile, and only the fabric-meter configuration contained one, so the default build stayed green while that example was broken. Checking both configurations is what caught it. The gate covers default features only. Under `rayon fabric-meter` this crate is also clean, but clippy reports jouleclaw-backend-apple through the path dependency, and that crate has 43 warnings -- 27 `unused_unsafe` and 15 `unused_imports` that are unused in THAT configuration and needed in others. Those want cfg work, not deletion, which is the trap #373 already paid for once. Verified: four lint specs pass, 343 loader-gguf tests, 77 omni tests. Co-Authored-By: Claude Opus 5 --- .github/workflows/standards-build.yml | 11 + apply-suggestions.py | 100 ---- census.py | 22 - .../benches/matmul_q4_k.rs | 5 + .../benches/matmul_q56_k.rs | 5 + .../benches/matmul_q8_0.rs | 5 + .../examples/fabric_calibrate.rs | 2 +- .../examples/mpp_bt_bench.rs | 5 +- .../examples/mpp_energy.rs | 2 +- .../jouleclaw-loader-gguf/src/attention.rs | 12 +- .../src/devstral_resident.rs | 6 +- .../src/fabric_dispatch.rs | 5 +- .../jouleclaw-loader-gguf/src/hybrid_lm.rs | 2 +- .../src/kv_cache_inplace.rs | 518 +++++++++--------- .../crates/jouleclaw-loader-gguf/src/mlx.rs | 4 +- .../jouleclaw-loader-gguf/src/openvino.rs | 18 +- .../src/safetensors_reader.rs | 8 +- .../jouleclaw-loader-gguf/src/synthetic.rs | 2 +- .../jouleclaw-loader-gguf/tests/dequant.rs | 5 + 19 files changed, 326 insertions(+), 411 deletions(-) delete mode 100644 apply-suggestions.py delete mode 100644 census.py diff --git a/.github/workflows/standards-build.yml b/.github/workflows/standards-build.yml index d76353d29..51310f697 100644 --- a/.github/workflows/standards-build.yml +++ b/.github/workflows/standards-build.yml @@ -594,4 +594,15 @@ jobs: # behind `cfg(feature = "fabric")` was deleted on exactly that # evidence and broke the fabric-meter build in this very job. lint jouleclaw/jouleclaw-rs -p jouleclaw-omni --features metal + # And the crate omni loads its weights through, which was linted + # by nothing at all: 161 warnings when first measured, now 0. + # + # Default features only. Under `rayon fabric-meter` this crate is + # still clean, but clippy also reports jouleclaw-backend-apple + # through the path dependency (`--no-deps` does not exclude + # workspace members), and that crate has 43 warnings of its own -- + # mostly `unused_unsafe` and `unused_imports` that are unused in + # THAT configuration and needed in others, so they want cfg work + # rather than deletion. + lint jouleclaw/jouleclaw-rs -p jouleclaw-loader-gguf lint sandbox/sandbox-rs -p sandbox-meter -p sandbox-proof diff --git a/apply-suggestions.py b/apply-suggestions.py deleted file mode 100644 index 536b4a8bc..000000000 --- a/apply-suggestions.py +++ /dev/null @@ -1,100 +0,0 @@ -#!/usr/bin/env python3 -"""Apply clippy's own suggested replacements for a whitelist of lints. - -`cargo clippy --fix` applies only MachineApplicable suggestions. Most of the -remaining lints here carry MaybeIncorrect ones, which are still exact spans -with exact replacement text -- they are "maybe incorrect" because clippy will -not promise semantics, not because the text is wrong. So apply them, then let -the crate's own tests decide. - - apply-suggestions.py [ ...] -""" -import json, re, subprocess, sys, collections, pathlib - -feats = sys.argv[1] -want = set(sys.argv[2:]) -cmd = ["cargo","clippy","--manifest-path","jouleclaw/jouleclaw-rs/Cargo.toml", - "-p","jouleclaw-loader-gguf","--all-targets","--no-deps","--keep-going", - "--message-format=json"] -if feats: - cmd += ["--features", feats] -proc = subprocess.run(cmd, capture_output=True, text=True) - -# file -> list of (byte_start, byte_end, replacement, code) -edits = collections.defaultdict(list) -seen = set() -skipped = collections.Counter() -for line in proc.stdout.splitlines(): - try: - m = json.loads(line) - except json.JSONDecodeError: - continue - if m.get("reason") != "compiler-message": - continue - d = m.get("message", {}) - if d.get("level") != "warning": - continue - code = ((d.get("code") or {}).get("code") or "").replace("clippy::", "") - if code not in want: - continue - # Collect every span in the message that carries a replacement. - def walk(msg): - for sp in msg.get("spans", []): - if sp.get("suggested_replacement") is not None: - yield sp - for ch in msg.get("children", []): - yield from walk(ch) - got = list(walk(d)) - if not got: - skipped[code] += 1 - continue - for sp in got: - repl = sp["suggested_replacement"] - # Some suggestions are templates, not replacements: - # `needless_range_loop` emits `for (i, ) in ...`, leaving the - # binding name AND the body's `w[i]` uses for a human. Applying one - # literally produces `` in the source and will not parse. - # A template placeholder is a bare lowercase word in angle brackets, - # like ``. A turbofish is not: `as_chunks::<2>()` is a real - # suggestion and the first version of this guard rejected all 28 of - # them. Reject only an angle-bracketed word that no `::` precedes. - if re.search(r"(?", repl): - skipped[code] += 1 - continue - key = (sp["file_name"], sp["byte_start"], sp["byte_end"]) - if key in seen: - continue - seen.add(key) - edits[sp["file_name"]].append( - (sp["byte_start"], sp["byte_end"], sp["suggested_replacement"], code)) - -applied = collections.Counter() -# clippy reports paths relative to the workspace manifest dir, not the repo root. -WS = pathlib.Path("jouleclaw/jouleclaw-rs") -for fn, es in edits.items(): - p = pathlib.Path(fn) - if not p.exists(): - p = WS / fn - if not p.exists(): - print(f" MISSING {fn}") - continue - data = p.read_bytes() - # Apply from the end so earlier offsets stay valid; drop overlaps. - es.sort(key=lambda e: e[0], reverse=True) - last_start = None - for start, end, repl, code in es: - if last_start is not None and end > last_start: - skipped[code] += 1 - continue - data = data[:start] + repl.encode() + data[end:] - last_start = start - applied[code] += 1 - p.write_bytes(data) - -print(f"applied {sum(applied.values())} edits in {len(edits)} files") -for c, n in applied.most_common(): - print(f" {n:4d} {c}") -if skipped: - print("no-suggestion or overlapping, left alone:") - for c, n in skipped.most_common(): - print(f" {n:4d} {c}") diff --git a/census.py b/census.py deleted file mode 100644 index e1aef0349..000000000 --- a/census.py +++ /dev/null @@ -1,22 +0,0 @@ -import json, subprocess, sys, collections -feats = sys.argv[1] if len(sys.argv)>1 else "" -cmd=["cargo","clippy","--manifest-path","jouleclaw/jouleclaw-rs/Cargo.toml", - "-p","jouleclaw-loader-gguf","--all-targets","--no-deps","--keep-going","--message-format=json"] -if feats: cmd += ["--features", feats] -pr=subprocess.run(cmd,capture_output=True,text=True) -c=collections.Counter(); e=0; seen=set() -for l in pr.stdout.splitlines(): - try: m=json.loads(l) - except Exception: continue - if m.get("reason")!="compiler-message": continue - d=m.get("message",{}); lv=d.get("level") - if lv not in ("warning","error"): continue - code=(d.get("code") or {}).get("code") or "(uncoded)" - sp=next((s for s in d.get("spans",[]) if s.get("is_primary")),None) - k=(code, sp.get("file_name") if sp else None, sp.get("line_start") if sp else None) - if k in seen: continue - seen.add(k) - if lv=="error": e+=1; continue - c[code]+=1 -print(f"features={feats or 'default'} errors={e} warnings={sum(c.values())}") -for k,v in c.most_common(): print(f" {v:4d} {k}") diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q4_k.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q4_k.rs index 24939142e..ec36e3720 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q4_k.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q4_k.rs @@ -5,6 +5,11 @@ //! models (Q4_K_M). These numbers + the wave-24 Q8_0 numbers //! tell the full quantized-matmul story on Apple Silicon. +// Benches and integration tests are their own crates, so the decision +// recorded in src/lib.rs does not reach them: these loops fill arrays by +// index, which is what the index is for. +#![allow(clippy::needless_range_loop)] + use criterion::{ black_box, criterion_group, criterion_main, BenchmarkId, Criterion, Throughput, }; diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q56_k.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q56_k.rs index 20ff87e63..efac28bc0 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q56_k.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q56_k.rs @@ -1,5 +1,10 @@ //! Criterion bench for Q5_K + Q6_K matmul kernels (wave 28). +// Benches and integration tests are their own crates, so the decision +// recorded in src/lib.rs does not reach them: these loops fill arrays by +// index, which is what the index is for. +#![allow(clippy::needless_range_loop)] + use criterion::{ black_box, criterion_group, criterion_main, BenchmarkId, Criterion, Throughput, }; diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q8_0.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q8_0.rs index 7895c46be..f8e3b8a40 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q8_0.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/benches/matmul_q8_0.rs @@ -6,6 +6,11 @@ //! wave-12 prefetch wins show up as steady-state throughput //! that's stable as `n` grows past the L2 cache. +// Benches and integration tests are their own crates, so the decision +// recorded in src/lib.rs does not reach them: these loops fill arrays by +// index, which is what the index is for. +#![allow(clippy::needless_range_loop)] + use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion, Throughput}; use jouleclaw_loader_gguf::matmul_q8_0::{ matmul_q8_0, matmul_q8_0_scalar, Q8_0_BLOCK_BYTES, Q8_0_ELEMS, diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/fabric_calibrate.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/fabric_calibrate.rs index a9f170eb1..b7eb7b965 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/fabric_calibrate.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/fabric_calibrate.rs @@ -162,7 +162,7 @@ fn main() { // Synthetic BF16 weight + activation. let mut w = vec![0u8; 2 * n * k]; - for (i, c) in w.chunks_exact_mut(2).enumerate() { + for (i, c) in w.as_chunks_mut::<2>().0.iter_mut().enumerate() { let v = (((i * 40503) % 2000) as f32 / 1000.0 - 1.0) * 0.05; c.copy_from_slice( &jouleclaw_loader_gguf::matmul_bf16::f32_to_bf16_rne(v).to_le_bytes(), diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/mpp_bt_bench.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/mpp_bt_bench.rs index 41f13cbfc..7789bb066 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/mpp_bt_bench.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/mpp_bt_bench.rs @@ -6,7 +6,7 @@ fn main() { use std::time::Instant; fn f32_to_bf16(v: f32) -> u16 { let b = v.to_bits(); - (((b + (((b >> 16) & 1) + 0x7fff)) >> 16)) as u16 + ((b + (((b >> 16) & 1) + 0x7fff)) >> 16) as u16 } // (M tokens, N out, K in) let shapes = [ @@ -38,7 +38,8 @@ fn main() { let mut neon = vec![0f32; m * n]; let t1 = Instant::now(); for _ in 0..iters { - jouleclaw_loader_gguf::matmul_bf16::matmul_bf16_threaded(&a, &w, &[], m, n, k, &mut neon); + jouleclaw_loader_gguf::matmul_bf16::matmul_bf16_threaded(&a, &w, &[], m, n, k, &mut neon) + .expect("threaded bf16 matmul"); } let neon_ms = t1.elapsed().as_secs_f64() * 1e3 / iters as f64; diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/mpp_energy.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/mpp_energy.rs index 2d4bbc458..081ba33c4 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/mpp_energy.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/examples/mpp_energy.rs @@ -27,7 +27,7 @@ fn main() { MemberRuns, NotMeasured, }; use std::time::{Duration, Instant}; - fn f32_to_bf16(v: f32) -> u16 { let b = v.to_bits(); (((b + (((b >> 16) & 1) + 0x7fff)) >> 16)) as u16 } + fn f32_to_bf16(v: f32) -> u16 { let b = v.to_bits(); ((b + (((b >> 16) & 1) + 0x7fff)) >> 16) as u16 } /// Write nothing: say why, with the IOReport status, on stderr and exit non-zero. fn refuse(path: &str, why: impl std::fmt::Display) -> ! { eprintln!("{}", refusal(path, why, jouleclaw_energy::ioreport_status())); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/attention.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/attention.rs index a6065b123..cfe6c59cd 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/attention.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/attention.rs @@ -272,6 +272,12 @@ pub fn attention_forward_f32_flash( /// touched. pub const FLASH_KV_CHUNK: usize = 256; +// Checked at compile time rather than in a test: a test only holds if someone +// runs it, and these are properties of a constant. Not required by the +// algorithm, but desirable for SIMD alignment when a NEON variant lands. +const _: () = assert!(FLASH_KV_CHUNK.is_power_of_two()); +const _: () = assert!(FLASH_KV_CHUNK >= 64); + fn as_u8(s: &[f32]) -> &[u8] { // SAFETY: f32 is plain-old-data; reinterpret to bytes // for the prefetch byte-offset API. Lifetime bound to s. @@ -479,9 +485,7 @@ mod tests { #[test] fn flash_kv_chunk_constant_is_power_of_two() { - // Not required by the algorithm, but desirable for SIMD - // alignment when a future NEON variant lands. - assert!(FLASH_KV_CHUNK.is_power_of_two()); - assert!(FLASH_KV_CHUNK >= 64); + // Both invariants are now checked at compile time, below; this test + // is kept only so the intent is findable by name. } } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/devstral_resident.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/devstral_resident.rs index 54bc31048..f134af8f3 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/devstral_resident.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/devstral_resident.rs @@ -79,15 +79,15 @@ fn read_f32_tensor(model: &GgufModel, name: &str) -> Result, LoadError> fn dequant_to_f32(bytes: &[u8], dtype: GgmlType, n: usize) -> Vec { match dtype { GgmlType::F32 => bytes[..n * 4] - .chunks_exact(4) + .as_chunks::<4>().0.iter() .map(|c| f32::from_le_bytes([c[0], c[1], c[2], c[3]])) .collect(), GgmlType::F16 => bytes[..n * 2] - .chunks_exact(2) + .as_chunks::<2>().0.iter() .map(|c| f16_to_f32(u16::from_le_bytes([c[0], c[1]]))) .collect(), GgmlType::BF16 => bytes[..n * 2] - .chunks_exact(2) + .as_chunks::<2>().0.iter() .map(|c| f32::from_bits((u16::from_le_bytes([c[0], c[1]]) as u32) << 16)) .collect(), GgmlType::Q4_K => crate::dequant::dequantize_q4_k(bytes, n).unwrap_or_else(|_| vec![0.0; n]), diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/fabric_dispatch.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/fabric_dispatch.rs index 408827034..55de7f7a7 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/fabric_dispatch.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/fabric_dispatch.rs @@ -1145,8 +1145,7 @@ mod enabled { // measured µJ/GFLOP prices it. Skipping it silently // under-credits the GPU and inflates the CPU's share. let hd = kv.hd(); - if hd > 0 { - let n_q = weights[0].1 / hd; + if let Some(n_q) = weights[0].1.checked_div(hd) { let seq = kv.len(); ledger_record("local/metal-gpu", n_q, seq, hd); ledger_record("local/metal-gpu", n_q, seq, hd); @@ -1203,7 +1202,7 @@ mod enabled { // The kernel needs N%32 and K%32; M is padded up to a // multiple of 64 (real prompt lengths aren't %64). Tiny // problems aren't worth the GPU round trip. - if !eligible || n % 32 != 0 || k % 32 != 0 || m * n < 64 * 128 { + if !eligible || !n.is_multiple_of(32) || !k.is_multiple_of(32) || m * n < 64 * 128 { return false; } debug_assert_eq!(a.len(), m * k); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/hybrid_lm.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/hybrid_lm.rs index 471cb3e19..531f3525a 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/hybrid_lm.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/hybrid_lm.rs @@ -114,7 +114,7 @@ impl GenerateParams { pub fn generate(model: &M, prompt: &[u32], params: &GenerateParams) -> Vec { assert!(!prompt.is_empty(), "empty prompt"); let mut state = model.new_state(); - let mut sampling = params.sampling.clone(); + let sampling = params.sampling.clone(); let mut history: Vec = prompt.to_vec(); let mut out = Vec::new(); let mut logits = model.forward_prompt(prompt, &mut state); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/kv_cache_inplace.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/kv_cache_inplace.rs index 0f1ac53b5..1f3727740 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/kv_cache_inplace.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/kv_cache_inplace.rs @@ -746,265 +746,7 @@ unsafe fn dequantize_row_neon(q: &[i8], scale: f32, out: &mut [f32]) { } } -#[cfg(test)] -mod kv_quant_tests { - use super::*; - #[test] - fn quant_dequant_roundtrip_is_bounded() { - // Synthetic K-like values in [-2, 2] over 4 rows of d_head=8. - let n_rows = 4; - let d_head = 8; - let mut fp32_bytes = Vec::with_capacity(n_rows * d_head * 4); - for r in 0..n_rows { - for i in 0..d_head { - let v = ((r * d_head + i) as f32) * 0.13 - 1.5; - fp32_bytes.extend_from_slice(&v.to_le_bytes()); - } - } - - let (q, scales) = quantize_per_row(&fp32_bytes, n_rows, d_head); - let back = dequantize_per_row(&q, &scales, n_rows, d_head); - - // Error per element <= scale/2 (theoretical bound on symmetric - // round-to-nearest quant). - let mut max_abs_err = 0.0_f32; - for r in 0..n_rows { - for i in 0..d_head { - let off = (r * d_head + i) * 4; - let orig = f32::from_le_bytes([ - fp32_bytes[off], fp32_bytes[off + 1], - fp32_bytes[off + 2], fp32_bytes[off + 3], - ]); - let deq = f32::from_le_bytes([ - back[off], back[off + 1], back[off + 2], back[off + 3], - ]); - let err = (orig - deq).abs(); - if err > max_abs_err { max_abs_err = err; } - assert!(err <= scales[r] / 2.0 + 1e-7, - "row {} col {}: err {} > bound {}", r, i, err, scales[r] / 2.0); - } - } - eprintln!("quant roundtrip max_abs_err = {max_abs_err:.6}"); - } - - #[test] - fn empty_cache_take_replace_roundtrip_int8_path() { - // Zero cache initially. take_buffers should yield zero fp32 - // tensors; replace_buffers with non-zero values should - // quantize+store; next take_buffers should dequant to ~the - // same values (within bound). - let mut cache = InPlaceKvCache::new_with_quant(1, 2, 4, 4, KvQuant::Int8); - let (k0, v0) = cache.take_buffers(0); - // All zero on a fresh cache. - for &b in k0.storage.bytes.iter() { assert_eq!(b, 0); } - for &b in v0.storage.bytes.iter() { assert_eq!(b, 0); } - - // Build a non-trivial fp32 K to put back. - let elems = 2 * 4 * 4; - let mut k_bytes = Vec::with_capacity(elems * 4); - for i in 0..elems { - let v = (i as f32) * 0.05 - 0.5; - k_bytes.extend_from_slice(&v.to_le_bytes()); - } - let k_new = Tensor { - meta: TensorMeta::new(Dtype::F32, &[2, 4, 4]), - storage: std::sync::Arc::new(TensorStorage { bytes: k_bytes.clone(), mapped: None }), - }; - cache.replace_buffers(0, k_new, v0); - - // Take again, dequant, compare. - let (k_back, _) = cache.take_buffers(0); - let mut max_err = 0.0_f32; - for i in 0..elems { - let off = i * 4; - let orig = f32::from_le_bytes([ - k_bytes[off], k_bytes[off + 1], k_bytes[off + 2], k_bytes[off + 3], - ]); - let deq = f32::from_le_bytes([ - k_back.storage.bytes[off], k_back.storage.bytes[off + 1], - k_back.storage.bytes[off + 2], k_back.storage.bytes[off + 3], - ]); - let e = (orig - deq).abs(); - if e > max_err { max_err = e; } - } - eprintln!("int8 cache roundtrip max_err = {max_err:.6}"); - // Per-row max_abs/127 bound; rows here have max_abs <= 1, so - // err per element <= 1/254 ≈ 0.004. - assert!(max_err < 0.01, "max_err too high: {max_err}"); - } - - #[test] - fn cache_bytes_reports_4x_savings() { - let fp32 = InPlaceKvCache::new(24, 8, 128, 64); - let int8 = InPlaceKvCache::new_with_quant(24, 8, 128, 64, KvQuant::Int8); - let ratio = fp32.cache_bytes() as f64 / int8.cache_bytes() as f64; - eprintln!( - "cache_bytes: fp32={} int8={} ratio={:.2}x", - fp32.cache_bytes(), int8.cache_bytes(), ratio, - ); - // ~4× — int8 trades 4× data for tiny scale overhead. - assert!(ratio > 3.5 && ratio < 4.5, "ratio {ratio} not ~4×"); - } - - /// Dequantize is pure integer-to-float multiply — NEON and scalar - /// must agree bit-for-bit (same f32 multiply, just lane-batched). - #[cfg(target_arch = "aarch64")] - #[test] - fn dequantize_row_neon_matches_scalar_bitwise() { - let d_head = 64; - let scale = 0.0123_f32; - // Cover the full int8 range a few times, plus tail elements - // that don't fit a 4-wide chunk. - let q: Vec = (0..d_head).map(|i| ((i as i32 * 7 - 3) & 0xFF) as i8).collect(); - let mut out_neon = vec![0.0_f32; d_head]; - let mut out_scalar = vec![0.0_f32; d_head]; - unsafe { super::dequantize_row_neon(&q, scale, &mut out_neon); } - super::dequantize_row_scalar(&q, scale, &mut out_scalar); - for (i, (&n, &s)) in out_neon.iter().zip(&out_scalar).enumerate() { - assert_eq!(n.to_bits(), s.to_bits(), - "row[{i}]: NEON {n} bits={:08x} vs scalar {s} bits={:08x}", - n.to_bits(), s.to_bits()); - } - } - - /// Standalone kernel bench: NEON vs scalar quant + dequant, no - /// allocator / cache / executor in the loop. Confirms the inner - /// SIMD kernel does the work it should — if this ratio is <1 (NEON - /// faster) but the full Bonsai bench is barely affected, the - /// bottleneck is somewhere else (likely allocation churn). - /// - /// Run: cargo test --release -p jouleclaw-loader-gguf - /// bench_quant_kernel -- --ignored --nocapture - #[cfg(target_arch = "aarch64")] - #[test] - #[ignore] - fn bench_quant_kernel() { - use std::time::Instant; - // Match Bonsai-1.7B: d_head=64, n_heads_kv * max_seq = 8 * 128 = 1024 rows. - let d_head = 64; - let n_rows = 1024; - let row_count = n_rows; - - // Pre-generate the input. - let mut rows = Vec::with_capacity(row_count * d_head); - for r in 0..row_count { - for i in 0..d_head { - let v = ((r * d_head + i) as f32) * 0.001 - 0.5; - rows.push(v); - } - } - let mut q_scalar = vec![0_i8; row_count * d_head]; - let mut q_neon = vec![0_i8; row_count * d_head]; - - // Warmup. - for r in 0..row_count { - let row = &rows[r * d_head..(r + 1) * d_head]; - let _ = super::quantize_row_scalar(row, &mut q_scalar[r * d_head..(r + 1) * d_head]); - } - for r in 0..row_count { - let row = &rows[r * d_head..(r + 1) * d_head]; - unsafe { - let _ = super::quantize_row_neon(row, &mut q_neon[r * d_head..(r + 1) * d_head]); - } - } - - let iters = 100; - - // Scalar quantize. - let t = Instant::now(); - for _ in 0..iters { - for r in 0..row_count { - let row = &rows[r * d_head..(r + 1) * d_head]; - let _ = super::quantize_row_scalar(row, &mut q_scalar[r * d_head..(r + 1) * d_head]); - } - } - let t_scalar_q = t.elapsed(); - - // NEON quantize. - let t = Instant::now(); - for _ in 0..iters { - for r in 0..row_count { - let row = &rows[r * d_head..(r + 1) * d_head]; - unsafe { - let _ = super::quantize_row_neon(row, &mut q_neon[r * d_head..(r + 1) * d_head]); - } - } - } - let t_neon_q = t.elapsed(); - - // Scalar dequantize. - let scale = 0.01; - let mut out_scalar = vec![0.0_f32; row_count * d_head]; - let mut out_neon = vec![0.0_f32; row_count * d_head]; - let t = Instant::now(); - for _ in 0..iters { - for r in 0..row_count { - super::dequantize_row_scalar( - &q_scalar[r * d_head..(r + 1) * d_head], scale, - &mut out_scalar[r * d_head..(r + 1) * d_head]); - } - } - let t_scalar_d = t.elapsed(); - - // NEON dequantize. - let t = Instant::now(); - for _ in 0..iters { - for r in 0..row_count { - unsafe { - super::dequantize_row_neon( - &q_scalar[r * d_head..(r + 1) * d_head], scale, - &mut out_neon[r * d_head..(r + 1) * d_head]); - } - } - } - let t_neon_d = t.elapsed(); - - let q_ratio = t_neon_q.as_secs_f64() / t_scalar_q.as_secs_f64(); - let d_ratio = t_neon_d.as_secs_f64() / t_scalar_d.as_secs_f64(); - eprintln!( - "[d_head={d_head}, n_rows={n_rows}, iters={iters}] \ - quant: scalar={:.3}ms neon={:.3}ms neon/scalar={:.2}x \ - dequant: scalar={:.3}ms neon={:.3}ms neon/scalar={:.2}x", - t_scalar_q.as_secs_f64() * 1000.0, - t_neon_q.as_secs_f64() * 1000.0, - q_ratio, - t_scalar_d.as_secs_f64() * 1000.0, - t_neon_d.as_secs_f64() * 1000.0, - d_ratio, - ); - } - - /// Quantize differs in rounding semantics — `vrndnq_f32` is round- - /// to-nearest-ties-to-even, `f32::round` is round-to-nearest-ties- - /// away-from-zero. They agree on every non-tie value; ties can - /// differ by ±1 in the i8 output. For typical fp values this - /// happens essentially never; we test that NEON is within ±1 of - /// scalar everywhere, and bit-exact on non-tie cases. - #[cfg(target_arch = "aarch64")] - #[test] - fn quantize_row_neon_within_one_ulp_of_scalar() { - let d_head = 64; - // Synthetic fp32 row with values that span [-2, 2], a mix of - // tie and non-tie quantize points. - let row: Vec = (0..d_head) - .map(|i| (i as f32) * 0.061 - 1.95) - .collect(); - let mut q_neon = vec![0_i8; d_head]; - let mut q_scalar = vec![0_i8; d_head]; - let s_neon = unsafe { super::quantize_row_neon(&row, &mut q_neon) }; - let s_scalar = super::quantize_row_scalar(&row, &mut q_scalar); - assert!((s_neon - s_scalar).abs() < 1e-6, "scales differ"); - let mut max_diff: i32 = 0; - for (i, (&n, &s)) in q_neon.iter().zip(&q_scalar).enumerate() { - let d = (n as i32 - s as i32).abs(); - if d > max_diff { max_diff = d; } - assert!(d <= 1, - "row[{i}]: NEON {n} vs scalar {s} differs by {d} (>1 ulp)"); - } - eprintln!("quantize_row_neon vs scalar max diff = {max_diff} ULP"); - } -} /// In-place decode-step graph. Inputs are token IDs + per-layer K/V buffers /// (always present, always full-sized); outputs are per-layer updated K/V @@ -1497,3 +1239,263 @@ pub fn build_head_only_graph( logits_output_name: "logits".into(), }) } + +#[cfg(test)] +mod kv_quant_tests { + use super::*; + + #[test] + fn quant_dequant_roundtrip_is_bounded() { + // Synthetic K-like values in [-2, 2] over 4 rows of d_head=8. + let n_rows = 4; + let d_head = 8; + let mut fp32_bytes = Vec::with_capacity(n_rows * d_head * 4); + for r in 0..n_rows { + for i in 0..d_head { + let v = ((r * d_head + i) as f32) * 0.13 - 1.5; + fp32_bytes.extend_from_slice(&v.to_le_bytes()); + } + } + + let (q, scales) = quantize_per_row(&fp32_bytes, n_rows, d_head); + let back = dequantize_per_row(&q, &scales, n_rows, d_head); + + // Error per element <= scale/2 (theoretical bound on symmetric + // round-to-nearest quant). + let mut max_abs_err = 0.0_f32; + for r in 0..n_rows { + for i in 0..d_head { + let off = (r * d_head + i) * 4; + let orig = f32::from_le_bytes([ + fp32_bytes[off], fp32_bytes[off + 1], + fp32_bytes[off + 2], fp32_bytes[off + 3], + ]); + let deq = f32::from_le_bytes([ + back[off], back[off + 1], back[off + 2], back[off + 3], + ]); + let err = (orig - deq).abs(); + if err > max_abs_err { max_abs_err = err; } + assert!(err <= scales[r] / 2.0 + 1e-7, + "row {} col {}: err {} > bound {}", r, i, err, scales[r] / 2.0); + } + } + eprintln!("quant roundtrip max_abs_err = {max_abs_err:.6}"); + } + + #[test] + fn empty_cache_take_replace_roundtrip_int8_path() { + // Zero cache initially. take_buffers should yield zero fp32 + // tensors; replace_buffers with non-zero values should + // quantize+store; next take_buffers should dequant to ~the + // same values (within bound). + let mut cache = InPlaceKvCache::new_with_quant(1, 2, 4, 4, KvQuant::Int8); + let (k0, v0) = cache.take_buffers(0); + // All zero on a fresh cache. + for &b in k0.storage.bytes.iter() { assert_eq!(b, 0); } + for &b in v0.storage.bytes.iter() { assert_eq!(b, 0); } + + // Build a non-trivial fp32 K to put back. + let elems = 2 * 4 * 4; + let mut k_bytes = Vec::with_capacity(elems * 4); + for i in 0..elems { + let v = (i as f32) * 0.05 - 0.5; + k_bytes.extend_from_slice(&v.to_le_bytes()); + } + let k_new = Tensor { + meta: TensorMeta::new(Dtype::F32, &[2, 4, 4]), + storage: std::sync::Arc::new(TensorStorage { bytes: k_bytes.clone(), mapped: None }), + }; + cache.replace_buffers(0, k_new, v0); + + // Take again, dequant, compare. + let (k_back, _) = cache.take_buffers(0); + let mut max_err = 0.0_f32; + for i in 0..elems { + let off = i * 4; + let orig = f32::from_le_bytes([ + k_bytes[off], k_bytes[off + 1], k_bytes[off + 2], k_bytes[off + 3], + ]); + let deq = f32::from_le_bytes([ + k_back.storage.bytes[off], k_back.storage.bytes[off + 1], + k_back.storage.bytes[off + 2], k_back.storage.bytes[off + 3], + ]); + let e = (orig - deq).abs(); + if e > max_err { max_err = e; } + } + eprintln!("int8 cache roundtrip max_err = {max_err:.6}"); + // Per-row max_abs/127 bound; rows here have max_abs <= 1, so + // err per element <= 1/254 ≈ 0.004. + assert!(max_err < 0.01, "max_err too high: {max_err}"); + } + + #[test] + fn cache_bytes_reports_4x_savings() { + let fp32 = InPlaceKvCache::new(24, 8, 128, 64); + let int8 = InPlaceKvCache::new_with_quant(24, 8, 128, 64, KvQuant::Int8); + let ratio = fp32.cache_bytes() as f64 / int8.cache_bytes() as f64; + eprintln!( + "cache_bytes: fp32={} int8={} ratio={:.2}x", + fp32.cache_bytes(), int8.cache_bytes(), ratio, + ); + // ~4× — int8 trades 4× data for tiny scale overhead. + assert!(ratio > 3.5 && ratio < 4.5, "ratio {ratio} not ~4×"); + } + + /// Dequantize is pure integer-to-float multiply — NEON and scalar + /// must agree bit-for-bit (same f32 multiply, just lane-batched). + #[cfg(target_arch = "aarch64")] + #[test] + fn dequantize_row_neon_matches_scalar_bitwise() { + let d_head = 64; + let scale = 0.0123_f32; + // Cover the full int8 range a few times, plus tail elements + // that don't fit a 4-wide chunk. + let q: Vec = (0..d_head).map(|i| ((i as i32 * 7 - 3) & 0xFF) as i8).collect(); + let mut out_neon = vec![0.0_f32; d_head]; + let mut out_scalar = vec![0.0_f32; d_head]; + unsafe { super::dequantize_row_neon(&q, scale, &mut out_neon); } + super::dequantize_row_scalar(&q, scale, &mut out_scalar); + for (i, (&n, &s)) in out_neon.iter().zip(&out_scalar).enumerate() { + assert_eq!(n.to_bits(), s.to_bits(), + "row[{i}]: NEON {n} bits={:08x} vs scalar {s} bits={:08x}", + n.to_bits(), s.to_bits()); + } + } + + /// Standalone kernel bench: NEON vs scalar quant + dequant, no + /// allocator / cache / executor in the loop. Confirms the inner + /// SIMD kernel does the work it should — if this ratio is <1 (NEON + /// faster) but the full Bonsai bench is barely affected, the + /// bottleneck is somewhere else (likely allocation churn). + /// + /// Run: cargo test --release -p jouleclaw-loader-gguf + /// bench_quant_kernel -- --ignored --nocapture + #[cfg(target_arch = "aarch64")] + #[test] + #[ignore] + fn bench_quant_kernel() { + use std::time::Instant; + // Match Bonsai-1.7B: d_head=64, n_heads_kv * max_seq = 8 * 128 = 1024 rows. + let d_head = 64; + let n_rows = 1024; + let row_count = n_rows; + + // Pre-generate the input. + let mut rows = Vec::with_capacity(row_count * d_head); + for r in 0..row_count { + for i in 0..d_head { + let v = ((r * d_head + i) as f32) * 0.001 - 0.5; + rows.push(v); + } + } + let mut q_scalar = vec![0_i8; row_count * d_head]; + let mut q_neon = vec![0_i8; row_count * d_head]; + + // Warmup. + for r in 0..row_count { + let row = &rows[r * d_head..(r + 1) * d_head]; + let _ = super::quantize_row_scalar(row, &mut q_scalar[r * d_head..(r + 1) * d_head]); + } + for r in 0..row_count { + let row = &rows[r * d_head..(r + 1) * d_head]; + unsafe { + let _ = super::quantize_row_neon(row, &mut q_neon[r * d_head..(r + 1) * d_head]); + } + } + + let iters = 100; + + // Scalar quantize. + let t = Instant::now(); + for _ in 0..iters { + for r in 0..row_count { + let row = &rows[r * d_head..(r + 1) * d_head]; + let _ = super::quantize_row_scalar(row, &mut q_scalar[r * d_head..(r + 1) * d_head]); + } + } + let t_scalar_q = t.elapsed(); + + // NEON quantize. + let t = Instant::now(); + for _ in 0..iters { + for r in 0..row_count { + let row = &rows[r * d_head..(r + 1) * d_head]; + unsafe { + let _ = super::quantize_row_neon(row, &mut q_neon[r * d_head..(r + 1) * d_head]); + } + } + } + let t_neon_q = t.elapsed(); + + // Scalar dequantize. + let scale = 0.01; + let mut out_scalar = vec![0.0_f32; row_count * d_head]; + let mut out_neon = vec![0.0_f32; row_count * d_head]; + let t = Instant::now(); + for _ in 0..iters { + for r in 0..row_count { + super::dequantize_row_scalar( + &q_scalar[r * d_head..(r + 1) * d_head], scale, + &mut out_scalar[r * d_head..(r + 1) * d_head]); + } + } + let t_scalar_d = t.elapsed(); + + // NEON dequantize. + let t = Instant::now(); + for _ in 0..iters { + for r in 0..row_count { + unsafe { + super::dequantize_row_neon( + &q_scalar[r * d_head..(r + 1) * d_head], scale, + &mut out_neon[r * d_head..(r + 1) * d_head]); + } + } + } + let t_neon_d = t.elapsed(); + + let q_ratio = t_neon_q.as_secs_f64() / t_scalar_q.as_secs_f64(); + let d_ratio = t_neon_d.as_secs_f64() / t_scalar_d.as_secs_f64(); + eprintln!( + "[d_head={d_head}, n_rows={n_rows}, iters={iters}] \ + quant: scalar={:.3}ms neon={:.3}ms neon/scalar={:.2}x \ + dequant: scalar={:.3}ms neon={:.3}ms neon/scalar={:.2}x", + t_scalar_q.as_secs_f64() * 1000.0, + t_neon_q.as_secs_f64() * 1000.0, + q_ratio, + t_scalar_d.as_secs_f64() * 1000.0, + t_neon_d.as_secs_f64() * 1000.0, + d_ratio, + ); + } + + /// Quantize differs in rounding semantics — `vrndnq_f32` is round- + /// to-nearest-ties-to-even, `f32::round` is round-to-nearest-ties- + /// away-from-zero. They agree on every non-tie value; ties can + /// differ by ±1 in the i8 output. For typical fp values this + /// happens essentially never; we test that NEON is within ±1 of + /// scalar everywhere, and bit-exact on non-tie cases. + #[cfg(target_arch = "aarch64")] + #[test] + fn quantize_row_neon_within_one_ulp_of_scalar() { + let d_head = 64; + // Synthetic fp32 row with values that span [-2, 2], a mix of + // tie and non-tie quantize points. + let row: Vec = (0..d_head) + .map(|i| (i as f32) * 0.061 - 1.95) + .collect(); + let mut q_neon = vec![0_i8; d_head]; + let mut q_scalar = vec![0_i8; d_head]; + let s_neon = unsafe { super::quantize_row_neon(&row, &mut q_neon) }; + let s_scalar = super::quantize_row_scalar(&row, &mut q_scalar); + assert!((s_neon - s_scalar).abs() < 1e-6, "scales differ"); + let mut max_diff: i32 = 0; + for (i, (&n, &s)) in q_neon.iter().zip(&q_scalar).enumerate() { + let d = (n as i32 - s as i32).abs(); + if d > max_diff { max_diff = d; } + assert!(d <= 1, + "row[{i}]: NEON {n} vs scalar {s} differs by {d} (>1 ulp)"); + } + eprintln!("quantize_row_neon vs scalar max diff = {max_diff} ULP"); + } +} diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/mlx.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/mlx.rs index efc9b7f88..70f004721 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/mlx.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/mlx.rs @@ -57,8 +57,8 @@ mod tests { #[test] fn mlx_re_exports_compile() { // Smoke test: the re-export names all resolve. - let _: Option = MlxDtype::from_str("F8_E8M0"); - let _: Option = MlxDtype::from_str("C64"); + let _: Option = MlxDtype::parse("F8_E8M0"); + let _: Option = MlxDtype::parse("C64"); // MlxError / MlxModel / etc. exist as the same types // — proven by the existing safetensors_reader tests. assert_eq!(MlxDtype::F8_E8M0.element_size(), 1); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/openvino.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/openvino.rs index 7bf62ddf8..84e6f7b45 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/openvino.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/openvino.rs @@ -89,7 +89,7 @@ pub enum OvDtype { impl OvDtype { /// Parse OpenVINO IR's `element_type` attribute value. /// Case-insensitive on the OpenVINO side. - pub fn from_str(s: &str) -> Self { + pub fn parse(s: &str) -> Self { match s.to_ascii_lowercase().as_str() { "f32" => Self::F32, "f16" => Self::F16, "bf16" => Self::BF16, "f64" => Self::F64, @@ -324,7 +324,7 @@ pub fn parse_openvino_xml( let val = attr.unescape_value().unwrap_or_default().to_string(); match attr.key.as_ref() { b"element_type" => { - cur_dtype = OvDtype::from_str(&val); + cur_dtype = OvDtype::parse(&val); } b"shape" => { cur_shape.clear(); @@ -437,14 +437,14 @@ mod tests { #[test] fn dtype_parse_round_trip() { - assert_eq!(OvDtype::from_str("f32"), OvDtype::F32); - assert_eq!(OvDtype::from_str("F32"), OvDtype::F32); // case-insensitive - assert_eq!(OvDtype::from_str("bf16"), OvDtype::BF16); - assert_eq!(OvDtype::from_str("nf4"), OvDtype::NF4); - assert_eq!(OvDtype::from_str("u1"), OvDtype::U1); - assert_eq!(OvDtype::from_str("f8e4m3"), OvDtype::F8E4M3); + assert_eq!(OvDtype::parse("f32"), OvDtype::F32); + assert_eq!(OvDtype::parse("F32"), OvDtype::F32); // case-insensitive + assert_eq!(OvDtype::parse("bf16"), OvDtype::BF16); + assert_eq!(OvDtype::parse("nf4"), OvDtype::NF4); + assert_eq!(OvDtype::parse("u1"), OvDtype::U1); + assert_eq!(OvDtype::parse("f8e4m3"), OvDtype::F8E4M3); assert!(matches!( - OvDtype::from_str("hypothetical_2030_dtype"), + OvDtype::parse("hypothetical_2030_dtype"), OvDtype::Unknown(_) )); } diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/safetensors_reader.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/safetensors_reader.rs index f9834d263..e868c3472 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/safetensors_reader.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/safetensors_reader.rs @@ -92,7 +92,7 @@ impl SafetensorsDtype { } /// Parse from the JSON dtype string. Returns `None` for /// unknown strings. - pub fn from_str(s: &str) -> Option { + pub fn parse(s: &str) -> Option { match s { "F32" => Some(Self::F32), "F16" => Some(Self::F16), @@ -284,7 +284,7 @@ pub fn parse_safetensors_mmap( name: key.clone(), reason: "missing dtype".into(), })?; - let dtype = SafetensorsDtype::from_str(dtype_str).ok_or_else(|| { + let dtype = SafetensorsDtype::parse(dtype_str).ok_or_else(|| { SafetensorsError::UnknownDtype(dtype_str.to_string()) })?; let shape_arr = t_obj @@ -505,7 +505,7 @@ mod tests { // MLX-specific additions (wave 35): "F8_E8M0", "C64", ] { - assert!(SafetensorsDtype::from_str(s).is_some(), "missing {s}"); + assert!(SafetensorsDtype::parse(s).is_some(), "missing {s}"); } } @@ -533,7 +533,7 @@ mod tests { #[test] fn dtype_rejects_unknown() { - assert!(SafetensorsDtype::from_str("WAT").is_none()); + assert!(SafetensorsDtype::parse("WAT").is_none()); } #[test] diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/synthetic.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/synthetic.rs index 39106c24b..e8a8b60a3 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/synthetic.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/src/synthetic.rs @@ -120,7 +120,7 @@ pub fn synthesize_llama_gguf(config: &SyntheticConfig) -> Vec { let dff = config.feed_forward_length; let v = config.vocab_size; // Under GQA: K and V have fewer heads. - let d_head = if config.head_count > 0 { d / config.head_count } else { d }; + let d_head = d.checked_div(config.head_count).unwrap_or(d); let d_kv = config.head_count_kv * d_head; let mut tensors: Vec<(String, Vec, GgmlType, Vec)> = Vec::new(); diff --git a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/dequant.rs b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/dequant.rs index 2a4c640bf..20da6fa3e 100644 --- a/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/dequant.rs +++ b/jouleclaw/jouleclaw-rs/crates/jouleclaw-loader-gguf/tests/dequant.rs @@ -2,6 +2,11 @@ //! hand-constructed blocks where the expected output is computable from //! the formula. +// Benches and integration tests are their own crates, so the decision +// recorded in src/lib.rs does not reach them: these loops fill arrays by +// index, which is what the index is for. +#![allow(clippy::needless_range_loop)] + use jouleclaw_loader_gguf::dequant::{dequantize_q4_k, dequantize_q5_k, dequantize_q8_0}; /// Encode an f32 as IEEE 754 binary16 little-endian. Just inverse of the -- 2.54.0 (Apple Git-157)