diff --git a/CHANGELOG.md b/CHANGELOG.md index b8032b5..4f31700 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,17 +1,60 @@ # Changelog -## Unreleased - -CUDA is a first-class target: the NIF now links correctly against it, and the -release publishes prebuilt CUDA artifacts for CUDA 12 and CUDA 13. - -Verified on 2x NVIDIA DGX Spark (GB10, `sm_121a`, aarch64, CUDA 13.0.2) against -this base (llama.cpp b10280) — a source build with `LLAMA_BACKEND=cuda` loads, -reports `backend: "CUDA"` from `LlamaCppEx.devices()`, offloads 31/31 layers to -the GPU, and passes the smoke suite: **528 tests, 0 failures**. +## v0.8.43 + +llama.cpp bump to b10362, on top of b10280 from v0.8.42, plus the CUDA work that +had been sitting unreleased: CUDA is now a first-class target — the NIF links +correctly against it, and the release publishes prebuilt CUDA artifacts for +CUDA 12 and CUDA 13. + +Unlike the v0.8.42 range, this one does **not** break the upstream C API. Every +change in it is additive, and no NIF source change was required. One of those +additions is a near miss worth naming rather than glossing over, because it has +the exact shape of the `load_mtp` trap that broke MTP in v0.8.41 — a new +`llama_context_params` field whose default is restrictive. See **Changed** for +why the conclusion differs this time, and why it was checked by running MTP +rather than only by reading the diff. + +Verified at this base (b10362) on an M1 Max, source builds with both +`LLAMA_BACKEND=metal` and `LLAMA_BACKEND=cpu`, each running the generation, +embedding and MTP suites against real GGUFs: **520 passed, 11 excluded** on both +(the exclusions are `:slow` and the known-broken `:mtp_cancel`). `mix credo +--strict`, `mix dialyzer` and `mix format --check-formatted` are clean, and a +source build from the Hex tarball resolves the pinned llama.cpp commit and links. + +The CUDA work in this release was verified separately on 2x NVIDIA DGX Spark +(GB10, `sm_121a`, aarch64, CUDA 13.0.2) against the previous base (llama.cpp +b10280) — a source build with `LLAMA_BACKEND=cuda` loads, reports +`backend: "CUDA"` from `LlamaCppEx.devices()`, offloads 31/31 layers to the GPU, +and passes the smoke suite: **528 tests, 0 failures**. ### Fixed +- **`max_tokens` was not an upper bound under MTP.** The verify loop emits up to + `1 + n_draft` tokens per iteration but checked the caller's budget only on + iteration entry, so the last iteration could run past it. `max_tokens: 16` + returned 16, 17 or 18 tokens for the same prompt under greedy decoding — + measured across six successive `generate/3` calls on one session. + + The overshoot is not constant because it is a function of how many drafts the + target accepts in the final iteration, and acceptance varies between runs on a + reused session (11, 10, 12, 11, 10, 12 accepted of 15 drafted, same prompt and + seed). The token *sequence* was deterministic throughout; only the stopping + point moved. That is what made it visible: `stream/3` and `generate/3` are the + same code path — `generate/3` is `stream_events/3` joined — yet they returned + different-length prefixes of the same continuation, which reads as a streaming + bug and is not one. + + The loop now re-checks the budget per token. Breaking mid-iteration leaves + positions decoded but not emitted, which the existing partial-accept rollback + already discards, so nothing else moved. `max_tokens: 16` now returns exactly + 16 tokens on every run, and a test pins the bound at 1, 4 and 16 rather than + only comparing the two entry points against each other. + + Pre-existing, not from this bump — the loop last changed in #79 (v0.8.39). It + survived because the `:mtp` suite has only ever run against one model, and the + boundary happened to land consistently there; a 0.8B MTP model with a + different acceptance profile exposes it on the first run. - **MTP hybrid rollback corrupted the KV cache after a partial accept.** This is a different bug from the `load_mtp` one fixed in v0.8.42, and the two are complementary: that one stopped the MTP layers being read off disk at all, @@ -117,6 +160,79 @@ the GPU, and passes the smoke suite: **528 tests, 0 failures**. - **Precompiler unit tests** — `test/precompiler_test.exs` pins the artifact selection rules, including the case that motivates the driver check. +### Changed + +- **llama.cpp submodule** — Updated from 61881b1f7 to 4801e3c56 (82 commits, tag + b10280 to b10362). No NIF source change was required: every API change in the + range is additive, and the binding builds all of its params from + `llama_*_default_params()` and uses only the public `llama_sampler_init_*` + constructors. + + - **`llama_context_params` gained `n_outputs_max_per_seq`** (#25532, multi-output + backend sampling), and its default in `llama_context_default_params()` is `1`, + not `0`. `llama_decode` enforces it and returns `-1` with + `backend sampling supports at most %u outputs per sequence` when a batch + exceeds it. + + This is the same shape as the `load_mtp` field that broke MTP in v0.8.41 — a + new restrictive default picked up silently from the defaults struct — and the + binding does request logits at every position of a sequence during MTP + prefill, which is exactly the pattern the limit forbids. It is nonetheless + inert here, for a reason worth writing down rather than rediscovering: the + check is gated on `!sampling.samplers.empty()` + (`src/llama-context.cpp:1664`), i.e. on *backend* samplers registered through + `llama_context_params.samplers`. This binding never sets that field — it + samples host-side via `llama_sampler_chain` and `llama_sampler_sample` — so + the map is empty and the limit is never applied. Because reading the diff is + how v0.8.41 got this wrong, it was also checked by running MTP end-to-end + against a real MTP GGUF rather than by inspection alone. + + If backend sampling is ever adopted here, `n_outputs_max_per_seq` becomes + load-bearing and must be set from `common_speculative_get_output_limits` + (new in this range) the way upstream's server and `speculative-simple` now do. + - **`llama_sampler_i` gained `backend_reset` and `copy_state`**, and + `backend_init` gained an `n_outputs_max_per_seq` parameter. This only affects + code that implements the sampler vtable itself; the NIF implements no custom + sampler. + - **New upstream entry points**, none currently used: `llama_sampler_copy`, + `common_sampler_copy`, `common_speculative_get_output_limits`, + `ggml_build_forward_order`. + - **Grammar semantics**: a repetition bound of `>= 2000` now degrades to + unbounded instead of raising (#26613). Reachable from `LlamaCppEx.Schema` and + `LlamaCppEx.Grammar` — a schema whose `maxItems` exceeded the threshold used + to fail grammar compilation with `number of repetitions exceeds sane + defaults` and now compiles. A `minItems` over the threshold still raises. + - **MTP/speculative upstream fixes**: memory allocation for MTP layers (#26605) + and MTP support for Nemotron (#26725). `common/speculative.h` is otherwise + additive and `common/chat.h` and `common/json-schema-to-grammar.h` are + untouched in this range. + - **Metal**: `NORM`/`RMS_NORM` fixed for row lengths that leave a partial + simdgroup (#26708), a `threadgroup` matrix instantiation removed from + `kernel_lightning_indexer` (#26646), and `ROLL` now requires a contiguous src + (#25928). All three are correctness fixes on the backend that ships in the + `aarch64-apple-darwin` artifact. + - **CPU/aarch64**: HWCAP fallbacks and fp16 variant detection (#25554), which + matters to the `LLAMA_PORTABLE=1` artifacts, plus a missing Q5_0 dispatch + (#26792) and an Android CPU-affinity fix (#26838). + - **CUDA**: `rms_norm + mul + rope` fusion (#26767) and a thread/block count + fix in the quantized cpy kernel launches (#26731). + - **ggml** version moved 0.18.1 to 0.19.0. No cmake option this Makefile passes + was renamed or removed, so the build configuration is unchanged. + - New architectures: Granite-Switch (#25107), Muse Glimmer (#26841), plus an + EXAONE 4.5 SWA fix (#26848). + +### Tests + +- **The `:mtp` and `:embeddings` suites now run against real models on Apple + Silicon**, not just the CPU-only generation model. `unsloth/Qwen3.5-0.8B-MTP-GGUF` + makes the MTP suite cheap enough to run per-release — the previously + recommended MTP GGUFs start at ~21 GB, which is why that suite had only ever + been exercised against a single model, which is how the `max_tokens` bound + above stayed broken. +- **`max_tokens` is pinned as an exact bound** at 1, 4 and 16 tokens, instead of + being covered only indirectly by a `stream/3`-versus-`generate/3` comparison + that could not distinguish a streaming bug from a budget bug. + ## v0.8.42 llama.cpp bump to b10280, on top of b10217 from v0.8.41. Unlike the last two diff --git a/Makefile b/Makefile index e6a7cc1..f629757 100644 --- a/Makefile +++ b/Makefile @@ -22,7 +22,7 @@ endif # Pinned llama.cpp commit, used when vendor/llama.cpp has to be cloned. MUST # match the vendor/llama.cpp submodule; bump both together, see # docs/release-guide.md. Override to build the NIF against another revision. -LLAMA_COMMIT ?= 61881b1f7f0b13d9e46d561fc25afcd6bbaec479 +LLAMA_COMMIT ?= 4801e3c567d5131dd41b387df5f2d4b1370d92be # The commit actually on disk. A submodule can be bumped without LLAMA_COMMIT # following it, and the build has to key off what is really there. diff --git a/README.md b/README.md index 2a43eec..9c543ca 100644 --- a/README.md +++ b/README.md @@ -603,6 +603,16 @@ Upstream llama.cpp implements more speculative types behind the same `common_spe - [`ggml-org/Qwen3.6-27B-MTP-GGUF`](https://huggingface.co/ggml-org/Qwen3.6-27B-MTP-GGUF) - [`unsloth/Qwen3.6-35B-A3B-MTP-GGUF`](https://huggingface.co/unsloth/Qwen3.6-35B-A3B-MTP-GGUF) +For trying MTP out, or for running the `:mtp` test suite, the small Qwen 3.5 MTP +builds are far cheaper than any of the above and carry the same `nextn` layers: + +- [`unsloth/Qwen3.5-0.8B-MTP-GGUF`](https://huggingface.co/unsloth/Qwen3.5-0.8B-MTP-GGUF) (`Q8_0`, ~0.8 GB — what this repo's MTP suite runs against) +- [`unsloth/Qwen3.5-4B-MTP-GGUF`](https://huggingface.co/unsloth/Qwen3.5-4B-MTP-GGUF) + +Acceptance on a 0.8B target is not representative of production throughput — +drafting is nearly as expensive as decoding at that size — so use these to +exercise the path, not to measure it. + A regular (non-MTP) Qwen 3.6 quant will fail at `LlamaCppEx.MTP.init/2` — the GGUF must contain the MTP head's tensors. To check a file before loading it, look for a `*.nextn_predict_layers` key and `blk.N.nextn.*` tensors in its metadata. The model must also be loaded with `load_mtp: true` (see below). Upstream gates those tensors behind a load-time flag that defaults to off, and they cannot be attached afterwards, so `MTP.init/2` refuses a model loaded without it rather than letting the omission surface later as `verify decode failed: code=-1`. diff --git a/c_src/llama_cpp_ex/llama_nif.cpp b/c_src/llama_cpp_ex/llama_nif.cpp index 766b10c..e80e17d 100644 --- a/c_src/llama_cpp_ex/llama_nif.cpp +++ b/c_src/llama_cpp_ex/llama_nif.cpp @@ -2128,7 +2128,18 @@ fine::Ok<> generate_mtp_tokens( bool eog = false; bool send_failed = false; - for (int i = 0; i < n_verify; i++) { + // `n_emitted < max_tokens` is re-checked here and not just by the outer + // `while`: this loop emits up to n_verify (= 1 + n_draft) tokens per + // iteration, so gating only on entry overshoots the caller's budget by + // up to n_draft - 1. Worse, the overshoot is not even constant --- how + // many tokens the final iteration emits depends on how many drafts the + // target accepts, which varies between runs on a reused session. That + // made `max_tokens: 16` return 16, 17 or 18 tokens for the same prompt + // under greedy decoding: the token *sequence* was deterministic, the + // stopping point was not. Breaking mid-iteration leaves + // n_accepted_total < n_verify, so the rollback below discards the + // positions that were decoded but never emitted. + for (int i = 0; i < n_verify && n_emitted < max_tokens; i++) { auto t0 = std::chrono::steady_clock::now(); sp.us_other.fetch_add( std::chrono::duration_cast(t0 - t_anchor).count(), diff --git a/docs/release-guide.md b/docs/release-guide.md index 6bf3361..7d22316 100644 --- a/docs/release-guide.md +++ b/docs/release-guide.md @@ -89,7 +89,7 @@ git -C "$d"/vendor/llama.cpp rev-parse HEAD # must equal the submodule SHA ## 4. Update version and changelog -1. **`mix.exs`** line 40: bump `@version` (e.g. `"0.6.5"` → `"0.6.6"`) +1. **`mix.exs`**: bump `@version` on `LlamaCppEx.MixProject` (e.g. `"0.8.42"` → `"0.8.43"`) 2. **`CHANGELOG.md`**: add a new `## vX.Y.Z` section at the top with: - The submodule commit range and count - Notable changes categorized by subsystem (follow existing format) diff --git a/mix.exs b/mix.exs index 75c9088..96cd81a 100644 --- a/mix.exs +++ b/mix.exs @@ -133,7 +133,7 @@ end defmodule LlamaCppEx.MixProject do use Mix.Project - @version "0.8.42" + @version "0.8.43" @source_url "https://github.com/nyo16/llama_cpp_ex" def project do diff --git a/test/mtp_model_test.exs b/test/mtp_model_test.exs index 22f6349..fa0e0a7 100644 --- a/test/mtp_model_test.exs +++ b/test/mtp_model_test.exs @@ -71,6 +71,28 @@ defmodule LlamaCppEx.MTPModelTest do assert {:ok, ^streamed} = MTP.generate(mtp, "Count to five:", opts) end + # Regression: the verify loop emits up to `1 + n_draft` tokens per iteration, + # and used to check the caller's budget only on iteration entry — so a request + # for 16 tokens came back with 16, 17 or 18 of them depending on how many + # drafts the target accepted in the final iteration. Acceptance varies between + # runs on a reused session, so this was observable as `stream/3` and + # `generate/3` returning different-length prefixes of the same greedy + # continuation. `max_tokens` is a bound, not a hint. + test "max_tokens is an exact upper bound regardless of draft acceptance", %{ + model: model, + session: mtp + } do + for max_tokens <- [1, 4, 16] do + assert {:ok, text} = + MTP.generate(mtp, "Count to twenty:", max_tokens: max_tokens, temp: 0.0) + + assert {:ok, tokens} = LlamaCppEx.Tokenizer.encode(model, text, add_special: false) + + assert length(tokens) <= max_tokens, + "max_tokens: #{max_tokens} produced #{length(tokens)} tokens: #{inspect(text)}" + end + end + test "stream_events/3 emits only documented events, ending with a terminal one", %{session: mtp} do events = mtp diff --git a/vendor/llama.cpp b/vendor/llama.cpp index 61881b1..4801e3c 160000 --- a/vendor/llama.cpp +++ b/vendor/llama.cpp @@ -1 +1 @@ -Subproject commit 61881b1f7f0b13d9e46d561fc25afcd6bbaec479 +Subproject commit 4801e3c567d5131dd41b387df5f2d4b1370d92be