Files
turboquant_plus/docs/upstream-pr-plan.md
T
693a391333 fix: address Codex + Gemini review findings
Codex fixes:
- wall_ms → wall_sec (BSD date doesn't support %3N milliseconds)
- Linux physical cores: use 'core id' count instead of processor count
- Remove unused $JSONFILE variable (caused crash on unset -u)
- Fix Metal shader comment accuracy (signs can mix per element)

Gemini/doc review fixes:
- Add inflection point detection (find_decode_inflection)
- Add 1K context measurement flagging (flag_unreliable_measurements)
- Add has_tensor impact to prediction model (+30% penalty)
- Use exponential cache fraction model (matches real data better)
- 4 new tests: inflection detection, unreliable measurement flagging

255 total tests passing (144 core + 71 diag + 40 replay).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: tturney@psyguard.ai
2026-03-26 09:34:29 -05:00

17 KiB

TurboQuant Upstream PR Plan — llama.cpp

Context

43 commits on feature/turboquant-kv-cache in our llama.cpp fork. 13,811 insertions across 29 files. Goal: get TurboQuant KV cache compression merged into ggml-org/llama.cpp.

Critical Upstream Requirements

AI Usage Policy (CRITICAL)

  • Project does NOT accept fully or predominantly AI-generated PRs
  • AI may only be used as an assistive tool (corrections, expanding verbose modifications)
  • Code initially generated by AI and subsequently edited is still considered AI-generated
  • AI-generated posts (bug reports, PR descriptions, discussions, responding to humans) are strictly prohibited
  • Repeated violations → permanent ban
  • If AI is used, you MUST:
    1. Explicitly disclose how AI was employed
    2. Perform comprehensive manual review before submitting
    3. Be prepared to explain every line of code to maintainers
  • See AGENTS.md for full policy

[!warning] Practical implication for us Disclose honestly (precedent: NVFP4 PR disclosed Claude Opus 4.6 usage and merged fine). The real bar is: be able to explain every line when a maintainer asks. Tom can — he built it. PR description and discussion posts must be written by Tom, not AI. The policy is aspirational; the enforcement is on understanding your code and honest disclosure.

Contributor levels:

  • Contributors (us): no special privileges, max 1 open PR
  • Collaborators (Triage): significant contributions, own parts of the code, review/maintain
  • Maintainers: review and merge PRs after code owner approval

Mandatory for new quantization types (ggml_type extension):

  1. GGUF model conversion — convert a small model, upload to HuggingFace
  2. Perplexity vs FP16/BF16 — llama-perplexity on wikitext-2 (native precision baseline)
  3. Perplexity vs similar-size types — compare to q4_0, q3_K, etc.
  4. KL divergence vs FP16/BF16 — llama-perplexity --kl-divergence (logit files: 11-37 GiB)
  5. KL divergence for similar-size types — same comparison set
  6. CPU performance — llama-bench on CPU only, compare to similar-size types
  7. Test cases — add to test-backend-ops for backend consistency (requires 2+ backends)
  8. Run full CI locally — bash ./ci/run.sh ./tmp/results ./tmp/mnt

Process rules:

  • Initial PR: CPU support only. Metal/CUDA in follow-up PRs.
  • Max 1 open PR for new contributors
  • Squash-merge by maintainers (<module> : <commit title> (#<issue_number>))
  • No third-party dependencies, extra files, extra headers
  • Cross-compatibility across OSes and architectures
  • Snake_case everywhere, 4-space indent, clang-format v15+
  • Use struct foo {} not typedef struct
  • Sized int types (int32_t) in public API
  • Enum values: UPPERCASE, prefixed with enum name
  • Naming: <class>_<method> where <method> is <action>_<noun>
  • Longest common prefix naming (e.g., number_small, number_big not small_number, big_number)
  • C/C++ filenames: lowercase with dashes, .h / .c / .cpp
  • Tensors store data in row-major order (dim 0 = columns, 1 = rows, 2 = matrices)
  • ggml_mul_mat(ctx, A, B) means C^T = A * B^T ⟺ C = B * A^T (unconventional!)

Community context:

  • Issue #20977 (feature request by mudler) and Discussion #20969 exist
  • JohannesGaessler raised concern: "Unless the PR author is known to be an expert, we shouldn't be adding more quantization formats"
  • Maintenance burden: new quant types increase binary size, compilation time, require coordinated backend extension across CUDA, Metal, Vulkan, etc.

Codex 5.4 Review Findings (Applied)

Codex reviewed the initial version of this plan and identified 6 critical issues:

  1. Quality not proven for exact submitted variant — the PPL 165.6 crisis is still in our docs. Must prove the exact CPU code path being submitted is quality-stable.
  2. GGUF requirement may not apply to KV-cache-only types — TurboQuant is a runtime cache type, not a model quantization format. Need to clarify with maintainers.
  3. Need Phase 0 (consensus) before Phase 1 (code) — don't burn the one-PR slot on unresolved design questions. Post in Issue #20977 first.
  4. Maintainer sponsorship is a blocker, not a nice-to-have — find a willing collaborator/maintainer to champion the PR before submitting.
  5. Commit structure too coarse — split graph changes from type/plumbing. Every commit must be buildable.
  6. Rotation data headers should be generated, not static — 12K+ lines of pre-computed data will get pushback. Generate deterministically in code.

All addressed in the revised plan below.


The Problem: Our Branch Is Metal-First

Our 43 commits are structured around Metal GPU optimization. Upstream wants CPU-first. We can't just clean up the branch and PR it — we need to restructure.

Additional problem (from Codex): CPU and Metal may not implement the same algorithm variant. CPU uses dense rotation in C code, Metal uses WHT. These produce numerically different results. The submitted CPU path must be self-consistent and independently validated.


Revised PR Strategy: 4-Phase Approach

Phase 0: Community Consensus (BLOCKING — before any code)

Goal: Get maintainer buy-in on the design before writing a single line of PR code.

Actions:

  1. Post in Issue #20977 with:
    • Exact algorithm variant (MSE-only PolarQuant, no QJL, block size 32, graph-side WHT rotation)
    • Block struct layout with byte-level detail
    • Benchmark table: PPL vs FP16, vs q8_0, vs q4_0, vs q3_K
    • KLD data
    • CPU llama-bench numbers
    • Open questions:
      • Does a KV-cache-only ggml_type require a GGUF model upload, or is a reproducible benchmark script sufficient?
      • Is deterministic rotation matrix generation acceptable, or do maintainers prefer a smaller lookup table?
      • Preferred naming: turbo3_0 vs tq3_0 vs something else?
    • Link to fork for anyone who wants to test
  2. Identify a willing collaborator/maintainer sponsor
  3. Wait for design feedback before proceeding

Exit criteria: At least one maintainer/collaborator has acknowledged the approach and not raised blockers.

Phase 1: Minimal CPU Reference (PR #1)

Goal: Get GGML_TYPE_TURBO3_0 accepted as a new KV cache type with CPU backend only.

Algorithm variant: Freeze on whatever is quality-proven on CPU. If block-32 + MSE-only + pre-rotate is validated on CPU with PPL confirmed, use that. If not, fall back to block-128 (last known-correct) and optimize in Phase 2.

What goes in:

  • ggml-common.h: block_turbo3_0 struct definition
  • ggml.h / ggml.c: GGML_TYPE_TURBO3_0 registration, type traits
  • ggml-turbo-quant.c: CPU quantize/dequantize (MSE-only, no QJL)
  • ggml-cpu/ops.cpp: CPU backend support for turbo3
  • src/llama-kv-cache.cpp: KV cache allocation for turbo3
  • src/llama-graph.cpp: Graph-side WHT rotation (pre-rotate-queries)
  • common/arg.cpp: --cache-type-k turbo3 --cache-type-v turbo3 CLI flags
  • tests/test-backend-ops.cpp: Backend consistency tests for turbo3
  • Rotation matrix generation code (deterministic, NOT static headers)

What stays out:

  • All Metal kernel code
  • All Metal-specific headers (turbo-wht.h, turbo-matrices.h)
  • GGML_OP_TURBO_WHT
  • Quality gate script
  • All documentation commits
  • Static rotation data headers (replaced by generation code)

Commit structure (4 commits, each buildable):

# Commit title Scope
1 ggml : add GGML_TYPE_TURBO3_0 type with block struct and traits Type enum, block_turbo3_0, type_traits, size/alignment
2 ggml : add TurboQuant CPU quantize and dequantize ggml-turbo-quant.c, rotation matrix generation, centroid tables
3 llama : add TurboQuant KV cache support with graph-side rotation KV cache init, graph rotation tensor, CLI flags
4 tests : add TurboQuant backend-ops and round-trip tests test-backend-ops entries, standalone round-trip test

Note: Maintainers squash-merge, so these become 1 commit. But during review, 4 buildable commits makes it reviewable.

Required deliverables:

Deliverable Status Notes
PPL vs FP16 on wikitext-2 ❌ Not yet Need f16 baseline, not just q8_0
PPL vs q8_0, q4_0, q3_K ❌ Not yet Similar-size comparison required
KL divergence vs FP16 ❌ Not yet Need 11-37 GiB base logits file
KL divergence vs q4_0, q3_K ❌ Not yet Same comparison set
CPU llama-bench vs q8_0, q4_0, q3_K ❌ Not yet Pure CPU, no Metal
GGUF model upload (if required) ❓ Clarify in Phase 0 May not apply to KV-cache-only types
test-backend-ops integration ❌ Not yet Requires 2+ backends
Full CI pass locally ❌ Not yet bash ./ci/run.sh
AI disclosure in PR description ❌ Draft needed Must be written by Tom, not AI
Rebase on latest master ❌ Not yet Do right before submitting
Maintainer/collaborator sponsor ❌ Not yet BLOCKING — from Phase 0
CPU path end-to-end validation ❌ Not yet Verify CPU dequant + graph rotation works without Metal

PR description guidance:

  • Written by Tom (human), not AI-generated
  • Neutral, technical tone — no marketing language
  • Lead with benchmark data, not claims
  • Don't say "within 1% of q8_0" — show the table and let reviewers judge
  • Reference the paper (arXiv 2504.19874) for algorithm details
  • Explicitly disclose AI usage: "Claude was used for codebase navigation and debugging. All code manually reviewed and tested."
  • Reference Issue #20977

Phase 2: Metal Backend (PR #2, after Phase 1 merges)

Goal: Add Metal GPU support for turbo3 KV cache.

What goes in:

  • Metal dequantize kernels (dequantize_turbo3_0, dequantize_turbo3_0_t4)
  • Flash attention template instantiations for turbo3
  • fp16 centroid LUT optimization
  • Float norm broadcast optimization
  • turbo-wht.h (Metal Fast Walsh-Hadamard)
  • GGML_OP_TURBO_WHT (custom graph op, if needed for Metal path)

Required deliverables:

  • Metal llama-bench results vs CPU and vs q8_0
  • Context scaling verification (2K-32K)
  • No regression on existing Metal tests
  • Numerical consistency: Metal and CPU produce same outputs (or documented/acceptable divergence)

Phase 3: Advanced Features (PR #3+, optional)

  • Layer-adaptive KV cache (per-layer precision)
  • turbo2/turbo4 types
  • Asymmetric K/V compression

Current Branch → PR Mapping

Source commits for Phase 1 (restructured, not cherry-picked):

The Phase 1 PR will be a clean extraction of CPU-only code from our branch, restructured for upstream conventions. This is necessary because:

  1. Our branch is Metal-first; Phase 1 is CPU-only
  2. Static rotation headers need to be replaced with generation code
  3. The exact algorithm variant must be frozen and validated on CPU first
  4. Code needs to follow upstream naming/style conventions (snake_case, etc.)

Key commits to reference for correctness:

  • 4ff5bf8: Core type definitions (block struct layout)
  • 099ceab: TURBO_D=128 independence from block size
  • a696962: Block size 32 format
  • 3032b23: MSE-only mode (no QJL)
  • 1f02172: Inverse rotation fix (critical quality bug)
  • 2bbe32f: Pre-rotate-queries approach (graph-side rotation logic)
  • ccbac3f: Optimized dequant (byte extraction patterns)

Commits to DROP entirely:

  • 12 pure documentation/investigation commits
  • 2 failed experiment commits (c6aa8ce, e40ca6f)

Commits to DEFER to Phase 2 (Metal):

  • bf0e223, 48da1d8, cf8ac72: Metal kernels and rotation matrices
  • 8e5e663, 8d682bd: Metal-specific fixes
  • 8aa0999: WHT foundation (Metal)
  • 69af339: GGML_OP_TURBO_WHT
  • 676f929, e4e0bde, 640e10e, c84e124: Graph WHT optimizations
  • a0e8a65, ccbac3f: Dequant optimizations (Metal path)
  • 654647a, aa6a3a1: Decode speed optimizations
  • 9cd0431: Quality gate script

Work Items — Ordered by Priority

BLOCKING (must complete before PR submission):

# Item Why it blocks Status
1 Phase 0: Post in Issue #20977 Can't PR without maintainer buy-in ❌
2 Find maintainer/collaborator sponsor Required for new ggml_type acceptance ❌
3 Freeze algorithm variant on CPU Must prove exact submitted code is quality-stable ❌
4 Replace static rotation headers with generation code 12K lines of static data will get rejected ❌
5 CPU end-to-end validation Verify CPU dequant + graph rotation works without Metal ❌
6 PPL vs FP16 baseline Mandatory deliverable ❌
7 PPL vs similar-size types (q4_0, q3_K) Mandatory deliverable ❌
8 KL divergence vs FP16 Mandatory deliverable (11-37 GiB logit files) ❌
9 KL divergence vs similar-size types Mandatory deliverable ❌
10 CPU llama-bench Mandatory deliverable ❌
11 test-backend-ops integration Mandatory for new ggml operators ❌
12 Full CI pass bash ./ci/run.sh ./tmp/results ./tmp/mnt ❌
13 Rebase on latest master Do right before submitting ❌
14 Human-written PR description AI-generated posts are prohibited ❌
15 AI disclosure statement Required by policy ❌

STRENGTHENING (improves acceptance odds):

# Item Impact
16 Multi-model PPL (Qwen + LLaMA) Shows generality
17 NIAH test results at various context lengths Demonstrates practical quality
18 Compare quality-per-bit vs q4_0 Justifies the maintenance burden
19 CODEOWNERS entry Signals long-term commitment
20 Clarify GGUF requirement for KV-cache-only types (Phase 0) Avoids cargo-culting checklist

Risk Assessment

Risk Severity Mitigation
"Too many quant types" rejection HIGH Phase 0 consensus + show quality-per-bit advantage over existing types
AI-generated code concern HIGH Clean rewrite by hand for PR. Honest disclosure. Explain every line.
CPU performance may be poor MEDIUM O(d²) rotation is per-batch, not per-token. Benchmark first. If bad, implement FWHT for CPU.
Rotation data size pushback MEDIUM Generate deterministically in code, not static headers
"Too invasive" — touches ggml type system AND llama graph MEDIUM Split commits cleanly. Each commit is minimal and buildable.
Block-32 quality not proven on CPU MEDIUM Fall back to block-128 if needed. Optimize later.
Cross-backend semantic divergence (CPU dense vs Metal WHT) MEDIUM Phase 1 is CPU-only so not an issue yet. Document for Phase 2.
KLD logit files are 11-37 GiB LOW Plan disk space ahead of time
Rebase conflicts (master moves fast) LOW Rebase right before submitting, keep PR small
No maintainer sponsor found HIGH If Phase 0 gets no traction, reconsider whether upstream is viable vs maintaining fork

Coding Style Checklist (from CONTRIBUTING.md)

Before submitting, verify all code follows:

  • Snake_case for functions, variables, types
  • 4-space indentation, brackets on same line
  • void * ptr, int & a spacing
  • struct foo {} not typedef struct foo {} foo
  • Omit optional struct/enum keywords in C++ (e.g., llama_context * ctx not struct llama_context * ctx)
  • Sized int types in public API (int32_t, not int)
  • Enum values: UPPERCASE, prefixed with enum name (GGML_TYPE_TURBO3_0)
  • Naming: <class>_<method> pattern, longest common prefix
  • C/C++ filenames: lowercase with dashes
  • No trailing whitespace
  • clang-format v15+ compatible
  • No third-party dependencies
  • Cross-platform compatible (no Apple-specific code in CPU path)
  • Vertical alignment for readability

Open Questions for Phase 0

  1. Does a KV-cache-only ggml_type require a GGUF model upload? Or is a reproducible benchmark script + reference model sufficient?
  2. Is deterministic rotation matrix generation (seeded PRNG → orthogonal matrix) acceptable? Or do maintainers prefer a different approach (e.g., Hadamard only, no random component)?
  3. Preferred type naming: GGML_TYPE_TURBO3_0 or GGML_TYPE_TQ3_0 or other?
  4. Is graph-side rotation (modifying build_attn to insert ggml_mul_mat for Q rotation) acceptable scope for a quantization PR? Or should it be a separate feature PR?
  5. Would maintainers prefer the rotation to be inside the dequant path (simpler graph, slower) or graph-side (faster, but touches more files)?

Summary

Old plan: 3 phases, Metal-first branch cleanup, static headers, marketing language in PR.

New plan: 4 phases starting with community consensus, clean CPU rewrite by hand, generated rotation data, neutral PR language, data-first presentation, maintainer sponsorship as a blocker.

The key insight from Codex: don't burn your one PR slot on unresolved design questions. Get buy-in first, then ship the smallest correct thing.