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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 71 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,12 @@ jobs:
# x86-64 CPU backend (no -march=native capture of the runner).
# SIMD dispatch for the CPU package needs a dlopen-path refactor
# of backend.cpp and is tracked separately.
cmake_extra: "-DGGML_NATIVE=OFF"
# This job also exercises GGML_LLAMAFILE=ON (project default).
# ⚠ Keep GGML_NATIVE=OFF until the dlopen refactor: on v0.20.x
# NATIVE is exclusive with GGML_BACKEND_DL, and flipping to
# NATIVE=ON + variants would unlink the CPU backend and break
# our direct symbols (see AGENT.md "ggml version gate").
cmake_extra: "-DGGML_NATIVE=OFF -DGAME_GGML_LLAMAFILE=ON"
Comment thread
KakaruHayate marked this conversation as resolved.
build_jobs: 4
pkg_ext: ""
lib_glob: "libggml*.so*"
Expand Down Expand Up @@ -311,6 +316,71 @@ jobs:
name: game-ggml-medium-model
path: model

- name: CPU inference smoke test (llamafile sgemm path)
if: matrix.backend == 'cpu' && runner.os == 'Linux'
run: |
set -euo pipefail
# Short 2 s 220 Hz sine, 44.1 kHz mono, PCM16 (stdlib only).
python3 - <<'PY'
import math, struct, wave
sr, dur = 44100, 2.0
n = int(sr * dur)
with wave.open("smoke.wav", "wb") as w:
w.setnchannels(1); w.setsampwidth(2); w.setframerate(sr)
frames = bytearray()
for i in range(n):
frames += struct.pack("<h", int(0.3 * 32767 * math.sin(2 * math.pi * 220 * i / sr)))
w.writeframes(bytes(frames))
PY

# Run F32 and Q8_0 models on the same wave. This exercises the
# default GGML_LLAMAFILE=ON mul_mat path (sgemm) with real model
# data, which --version never touches.
for m in model/game_medium.gguf model/game_medium_q8.gguf; do
tag=$(basename "$m" .gguf)
echo "== extract: $m (llamafile path) =="
build/bin/game_ggml_cli extract smoke.wav -m "$m" \
--output-formats mid,csv,txt --output-dir "out-$tag" \
--nsteps 8 --cache-threshold 0 --seed 42
test -s "out-$tag/smoke.mid"
test -s "out-$tag/smoke.csv"
done

# Cross-check F32 vs Q8_0 with the repo's own tolerances
# (PITCH_EPS=5 cents, TIME_EPS=0.05 s — same as verify_backends.py).
# Note-count drift of a couple notes is expected between F32 and
# Q8 (see docs/benchmark-7channel.md: CPU F32 143 vs Q8 145), so
# we require it be small (<5%) and compare pitch within 5 cents on
# the shared prefix using a midi-cents parse.
python3 - <<'PY'
import csv, re, sys
LETTER = {"C": 0, "D": 2, "E": 4, "F": 5, "G": 7, "A": 9, "B": 11}
def cents(p):
if p == "rest":
return None
m = re.fullmatch(r"([A-G])(#?)(\d+)([+-]\d+)?", p.strip())
if not m:
raise ValueError(f"unparsable pitch {p!r}")
letter, sharp, octave, off = m.groups()
return (LETTER[letter] + (1 if sharp else 0) + (int(octave) + 1) * 12) * 100 + int(off or 0)
def notes(path):
with open(path, newline="") as f:
return [(cents(r["pitch"]), float(r["offset"]), float(r["duration"])) for r in csv.DictReader(f)]
a, b = notes("out-game_medium/smoke.csv"), notes("out-game_medium_q8/smoke.csv")
if not a or not b:
print("MISMATCH: empty note list"); sys.exit(1)
if abs(len(a) - len(b)) > max(2, int(0.05 * max(len(a), len(b)))):
print(f"MISMATCH note count {len(a)} vs {len(b)}"); sys.exit(1)
for i, (pa, pb) in enumerate(zip(a, b)):
if (pa[0] is None) != (pb[0] is None):
print(f"MISMATCH note[{i}] rest-vs-pitch"); sys.exit(1)
if pa[0] is not None and abs(pa[0] - pb[0]) > 5:
print(f"MISMATCH note[{i}] pitch {pa[0]} vs {pb[0]}"); sys.exit(1)
if abs(pa[1] - pb[1]) > 0.05 or abs(pa[2] - pb[2]) > 0.05:
print(f"MISMATCH note[{i}] time {pa[1]:.2f}/{pa[2]:.2f} vs {pb[1]:.2f}/{pb[2]:.2f}"); sys.exit(1)
print(f"OK: {len(a)} notes, F32 vs Q8 within tolerance")
PY

- name: Package .oudep (full + quantized variants)
run: |
set -euo pipefail
Expand Down
159 changes: 159 additions & 0 deletions AGENT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
# AGENT.md — working notes for AI agents & contributors

Save future agents (and humans) from re-deriving the repo's non-obvious
constraints. If you plan to touch the build system, the ggml dependency, or
the CPU/GPU backend wiring, **read this first**.

---

## 1. ggml version gate — stay on v0.20.2, keep the CPU config frozen (hard rule)

The project pins **ggml `v0.20.2`** (`cmake/Dependencies.cmake`, URL archive)
and builds the CI CPU package as a **non-DL portable baseline with
`GGML_NATIVE=OFF`**. This is load-bearing:

- v0.20.x turned `GGML_CPU_ALL_VARIANTS` into **dlopen MODULE plugins**
(`GGML_BACKEND_DL`), and DL mode does **not** link the CPU backend into the
ggml umbrella target.
- Our `src/backend.cpp` calls the CPU backend API **directly**
(`ggml_backend_cpu_init`, `ggml_backend_cpu_set_n_threads`,
`ggml_threadpool_new`, `ggml_backend_cpu_set_threadpool`). With DL those
become `undefined reference` link errors.
- `GGML_NATIVE` and `GGML_BACKEND_DL` are mutually exclusive upstream
(ggml-cpu CMake `FATAL_ERROR`).

**Therefore:** do **not** flip the CI CPU job to NATIVE/ALL_VARIANTS and do
**not** downgrade the ggml tag to v0.19.0 "for GGML_NATIVE". Both are wrong
until `backend.cpp` is refactored to load the CPU backend via
`GGML_BACKEND_DL`. Only after that refactor is the gate eligible for review.

When the gate eventually lifts, re-verify:
1. Both in-repo patches apply to the new tag (`git apply --check` against the
fetched ggml source).
2. The CUDA arch list still passes ggml's CMake.
3. `src/backend.cpp` `ggml_version_string()` matches the new tag.
4. README/README_CN dependency table matches (it drifted before — see #4).

---

## 2. Direct CPU backend symbols are a linked dependency, not dlopen

`src/backend.cpp` calls ggml CPU backend API **directly**
(`ggml_backend_cpu_init`, `ggml_threadpool_new`, ...). This couples us to a
build where the CPU backend is PUBLIC-linked into the ggml umbrella target
(non-DL). That is the root cause of pitfall #1. Any change that turns the
CPU backend into a dlopen module must come with the backend.cpp refactor.

---

## 3. In-repo ggml patches are load-bearing (verify before removing)

Two ggml patches live in `cmake/patches/` and are re-applied by
`cmake/Dependencies.cmake` `game_ggml_apply_patch` (idempotent; fails if the
diff stops applying):

- `ggml-metal-binary-archive.patch` — Metal first-run <1s (MTLBinaryArchive
PSO cache). Pairs with `GGML_METAL_EMBED_LIBRARY OFF`.
- `ggml-vulkan-pipeline-cache.patch` — Vulkan cold-start PSO persistence
(disk-backed `VkPipelineCache`).

They are **project-owned** forks of upstream, not part of ggml main. Do not
delete them; keep their `.md` baseline per anchor tag.

---

## 4. Doc / pin drift is a known trap

Both READMEs previously said ggml `v0.11.0` while the pin was `v0.19.0` (then
`v0.20.2`). Always cross-check `cmake/Dependencies.cmake` against the docs;
never trust the README version row by itself. Keep both README dependency
tables in sync with the actual pin.

---

## 5. CPU ISA: NATIVE is a headline, not the whole story

- `GGML_NATIVE` adds `-march=native` (GCC/Clang) or MSVC `FindSIMD`.
- It does NOT flip ggml's hand-written SIMD kernels — those key off
`GGML_AVX*/GGML_AVX512*` options. Real AVX-512 use requires enabling e.g.
`-DGGML_AVX512=ON -DGGML_AVX512_VNNI=ON`.
- `GGML_LLAMAFILE` (project option `GAME_GGML_LLAMAFILE`, default ON) routes
CPU `mul_mat` through llamafile `sgemm` (tinyBLAS) for Q8_0/F32/BF16 on
AVX2+. Trade-off: it changes the FP summation order → CPU bit-exactness
tests must be re-run when toggled, and the CI cpu job validates the combo.

---

## 6. CUDA arch list + toolkit coupling

The CI CUDA arch list `75;80;86;89;90;120-virtual` requires CUDA 12.8+ for
Blackwell (`sm_100`/`sm_120`) targets; `compute_120` PTX gives RTX 50-series
JIT. Older toolkits can't compile `120-virtual` — trim the list rather than
"fixing" by touching NATIVE (the CUDA job already correctly uses
`GGML_NATIVE=OFF`; unrelated).

---

## 7. CPU/macOS cross-arch footguns

- CI `linux-x64-cpu` is a **portable `GGML_NATIVE=OFF` baseline** and must
keep working across runner CPU generations (do not rely on NATIVE capture;
the `_deps` cache key includes `Dependencies.cmake`).
- `macos-x64-metal` cross-compile sets `GGML_NATIVE=OFF` — otherwise the CPU
backend detects the ARM host (`apple-m1`) and fails.
- NVCC/VS version coupling on Windows is documented in BUILDING.md; don't
modernize blindly.

---

## 8. DBCache & FP drift — the segmenter is deliberately approximate

`--nsteps > 1` engages DBCache (cross-step tail reuse). It is near-lossless
on purpose; the **device-side** decision metric exists specifically to avoid
host round-trips on GPU. Changing the threshold/defaults/robustness knobs
silently changes note output — measure frame-level metrics, not just note
count, before "improving" it. On Vulkan, Q8_0 can flip a boundary note vs
F32; bit-consistent users use F32.

---

## 9. Front-end & backend parallelism live in two places

`src/mel.cpp` uses its own small C++ thread pool + batched pocketfft; CPU
graph compute uses a persistent ggml threadpool (hybrid polling). They are
independent. Keep the mel pool ≤ 8 threads and guard by frame count (short
clips shouldn't spawn threads).

---

## 10. Don't fight the sandbox for writes outside the repo

The git metadata of a checkout may live in a **different directory than the
worktree** (linked worktree; `git rev-parse --git-dir` finds it). Git index
writes (branch/stash/commit) write to that directory, not the worktree. If a
git write fails with "Permission denied" on `index.lock`, resolve the actual
git dir with `git rev-parse --git-dir` (`--absolute-git-dir` for a full path),
and it's almost always the harness sandbox — **escalate once** with
`sandbox_permissions: danger-full-access` + justification; do not retry in a
loop. Never hard-code a local absolute path in committed docs.

---

## 11. Scripts are part of the release pipeline

`scripts/convert_pt_to_gguf.py`, the quant config, and the
benchmark/alignment scripts back CI's `prepare-model` and reproducible
benches. Deleting them breaks release packaging. If a branch PR shows
script deletions, they are probably an accident unless the PR message says
otherwise.

---

## Rules of thumb

- Prefer explicit `-D...=ON/OFF` at configure over env/`CMakeCache` surgery;
re-define a FetchContent dependency → delete `build/` and reconfigure.
- Any change to `cmake/Dependencies.cmake`, `cmake/patches/`, or CI flags
gets reviewed against the pitfalls above before pushing.
- When in doubt, rebase a small branch onto `origin/main` and let CI (and
CodeRabbit) review the exact delta — that is what this file is for.
19 changes: 19 additions & 0 deletions BUILDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,25 @@ This document covers how to build `game_ggml_cli` from source on Linux, macOS an
Windows. The project is self-contained — all dependencies are pulled via CMake
FetchContent at configure time.

> **⚠ CPU backend version/config gate — ggml `v0.20.2` + `GGML_NATIVE=OFF`**
>
> The project runs **ggml `v0.20.2`** with a **non-DL portable baseline CPU
> package (`GGML_NATIVE=OFF`)**. This combination is load-bearing — changing
> it breaks linking:
>
> - v0.20.x turned CPU ISA variants into **dlopen MODULE plugins**
> (`GGML_CPU_ALL_VARIANTS` now hard-requires `GGML_BACKEND_DL`), and DL
> mode no longer links the CPU backend into the ggml umbrella target — so
> this project's direct `ggml_backend_cpu_init` / `ggml_threadpool_new` /
> `ggml_backend_cpu_set_threadpool` references fail to link.
> - `GGML_NATIVE` and `GGML_BACKEND_DL` are mutually exclusive upstream, so
> the portable non-DL CPU job must keep `GGML_NATIVE=OFF`.
> - Do **not** revert the ggml tag to v0.19.0 for GGML_NATIVE, and do **not**
> flip the CI CPU job to NATIVE/ALL_VARIANTS, until the `backend.cpp`
> dlopen refactor is complete.
>
> Rationale and the full pitfall list are in [`AGENT.md`](AGENT.md).

## Prerequisites

### Linux
Expand Down
3 changes: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ option(GAME_GGML_CUDA "Enable ggml CUDA backend" OFF)
option(GAME_GGML_VULKAN "Enable ggml Vulkan backend" OFF)
option(GAME_GGML_BUILD_CLI "Build game_ggml_cli executable" ON)
option(GAME_GGML_BUILD_TESTS "Build unit tests (requires GoogleTest via FetchContent)" OFF)
# Optional: use ggml-llamafile's tuned sgemm kernels for CPU mul_mat
# (Q8_0/F32/BF16, AVX2+). Off yields the stock ggml vec-dot kernels.
option(GAME_GGML_LLAMAFILE "Enable ggml-llamafile sgemm kernels (CPU)" ON)

# On Apple, default to Metal backend on. Use GAME_GGML_METAL to override.
if(APPLE)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ trees live under `build/_deps/<name>-src/` after the first configure.

| Dependency | Version pin | License | SPDX identifier |
|---|---|---|---|
| [ggml](https://github.com/ggerganov/ggml) | `v0.20.2` tag | MIT | MIT |
| [ggml](https://github.com/ggerganov/ggml) | `v0.20.2` tag (pin rationale + upgrade gate in [AGENT.md](AGENT.md) "ggml version gate") | MIT | MIT |
| [pocketfft](https://gitlab.mpcdf.mpg.de/mtr/pocketfft) | commit `32424d20` on `cpp` branch | BSD-3-Clause | BSD-3-Clause |
| [dr_libs](https://github.com/mackron/dr_libs) | commit `243e26ff` on `master` | Public Domain / MIT-0 (dual) | `Unlicense OR MIT-0` |
| [GoogleTest](https://github.com/google/googletest) | `v1.14.0` tag (tests only) | BSD-3-Clause | BSD-3-Clause |
Expand Down
2 changes: 1 addition & 1 deletion README_CN.md
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ ctest --test-dir ggml_backend/build --output-on-failure

| 依赖 | 版本 pin | 许可 | SPDX 标识 |
|---|---|---|---|
| [ggml](https://github.com/ggerganov/ggml) | `v0.20.2` tag | MIT | MIT |
| [ggml](https://github.com/ggerganov/ggml) | `v0.20.2` tag(pin 理由与升级门槛见 [AGENT.md](AGENT.md) "ggml version gate") | MIT | MIT |
| [pocketfft](https://gitlab.mpcdf.mpg.de/mtr/pocketfft) | `cpp` 分支 `32424d20` | BSD-3-Clause | BSD-3-Clause |
| [dr_libs](https://github.com/mackron/dr_libs) | `master` 分支 `243e26ff` | Public Domain / MIT-0(双许可) | `Unlicense OR MIT-0` |
| [GoogleTest](https://github.com/google/googletest) | `v1.14.0` tag(仅测试) | BSD-3-Clause | BSD-3-Clause |
Expand Down
17 changes: 15 additions & 2 deletions cmake/Dependencies.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ include(FetchContent)
set(FETCHCONTENT_UPDATES_DISCONNECTED ON CACHE BOOL "" FORCE)

# Propagate backend toggles as ggml's own option names *before* add_subdirectory.
# Note: GGML_LLAMAFILE deliberately does NOT use FORCE so an explicit
# -DGGML_LLAMAFILE=OFF survives; GAME_GGML_LLAMAFILE is the public switch.
set(GGML_BUILD_TESTS OFF CACHE BOOL "" FORCE)
set(GGML_BUILD_EXAMPLES OFF CACHE BOOL "" FORCE)
set(GGML_METAL ${GAME_GGML_METAL} CACHE BOOL "ggml: enable Metal" FORCE)
set(GGML_CUDA ${GAME_GGML_CUDA} CACHE BOOL "ggml: enable CUDA" FORCE)
set(GGML_VULKAN ${GAME_GGML_VULKAN} CACHE BOOL "ggml: enable Vulkan" FORCE)
set(GGML_LLAMAFILE ${GAME_GGML_LLAMAFILE} CACHE BOOL "ggml: llamafile sgemm kernels (CPU)")

# Apple cold-start optimisation: pre-compile a default.metallib instead of
# embedding source; paired with the binary-archive patch below this cuts ~7 s
Expand Down Expand Up @@ -82,8 +85,18 @@ if(GAME_GGML_VULKAN AND WIN32 AND NOT EXISTS "$ENV{VULKAN_SDK}/Lib/cmake/SPIRV-H
endif()

# ---------------------------------------------------------------------------
# ggml (MIT) — tensor engine.
# Fetched + Metal binary-archive patch applied on Apple for fast cold start.
# ggml (MIT) — tensor engine. Fetched + Metal binary-archive patch applied
# on Apple for fast cold start.
#
# > CPU version/config gate — see AGENT.md "ggml version gate". ggml pinned
# > at v0.20.2; the CI CPU package is non-DL + GGML_NATIVE=OFF (portable
# > baseline). Do NOT flip the CPU job to GGML_NATIVE / GGML_CPU_ALL_VARIANTS
# > and do NOT downgrade the tag until backend.cpp's direct references
# > (ggml_backend_cpu_init, ggml_threadpool_new, ggml_backend_cpu_set_threadpool)
# > are moved behind a GGML_BACKEND_DL dlopen load: on v0.20.x variants
# > require DL, DL unlinks the CPU backend from the umbrella target, and
# > NATIVE + DL are mutually exclusive upstream. NATIVE only matters after
# > GGML_AVX512* options are enabled; it does not flip ggml's SIMD kernels.
# ---------------------------------------------------------------------------
FetchContent_Declare(
ggml
Expand Down
Loading