diff --git a/CHANGELOG.md b/CHANGELOG.md index de68812..597d7ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,22 @@ DGX Spark (GB10) support: a silent ARM code-generation bug fixed, the ggml RPC backend wired up so a model can span two machines, and a measured runbook for both configurations in [docs/dgx-spark.md](docs/dgx-spark.md). -llama.cpp bumped to [`b10435`](https://github.com/ggml-org/llama.cpp/releases/tag/b10435) -(`9e40df63b`), which brings Qwen 3.8 in under the existing `qwen35` -architecture, and MTP support for its target/sidecar split (see Added). - -Verified on macOS (Metal, no RPC): **428 passed, 149 excluded**, plus -**6 passed** with `--include mtp_sidecar` against Qwen3.8-27B-Q4_K_M and its -`mtp-*-Q4_0` head. On a DGX Spark (CUDA 13.0, `sm_121a`): **428 passed**. +llama.cpp bumped to [`b10582`](https://github.com/ggml-org/llama.cpp/releases/tag/b10582) +(`e85caa81e`), by way of b10435, which brought Qwen 3.8 in under the existing +`qwen35` architecture, and MTP support for its target/sidecar split (see Added). + +Verified on macOS (Metal) at `e85caa81e`, running **every** tag the suite +excludes by default. Default build: **428 passed, 149 excluded** with no model; +**569 passed, 8 excluded** for `--include smoke --include embeddings +--include slow --include mtp` (Qwen3.5-0.8B-UD-Q4_K_XL, +Qwen3-Embedding-0.6B-f16, Qwen3.6-35B-A3B-MTP-UD-Q4_K_XL); **434 passed** +for `--include mtp_sidecar` (Qwen3.8-27B-Q4_K_M plus its `mtp-*-Q4_0` head). +`LLAMA_RPC=1` build against a live local `LlamaCppEx.RPC.Server`: +**429 passed, 148 excluded** for `--include rpc_live`, and the model tags +re-run on that build give the same **569** and **434**. On a DGX Spark +(CUDA 13.0, `sm_121a`), measured at b10435: **428 passed**. + +The one tag that is not green is `:mtp_cancel`, and it moved: see Changed. ### Fixed @@ -149,6 +158,50 @@ Verified on macOS (Metal, no RPC): **428 passed, 149 excluded**, plus a device reports it cannot support it. The NIF always sets `load_mode` explicitly from `:use_mmap`/`:use_mlock`/`:use_direct_io`, so behaviour is unchanged and `:auto` is not exposed yet. +- **llama.cpp bumped to `e85caa81e`** (b10582), 147 commits past b10435, and + `LLAMA_COMMIT` moved with the submodule. Nothing in the binding changed: + `include/llama.h`, `ggml/include/ggml-backend.h`, `common/chat.h`, + `common/json-schema-to-grammar.h` and `common/speculative.h` are byte-identical + across the range, so `llama_nif.cpp` needed no edit — the diff that matters for + a bump is the header diff, and this one is empty. `ggml-rpc.h` moved only + `RPC_PROTO_MINOR_VERSION` 0 → 1 (5.0.0 → 5.1.0), for `use_count` propagation + that lets backends fuse ops behind RPC (`af5172627`); same major, no signature + change, and nothing in this repo pins the protocol version. + Two upstream commits land on the MTP path and both are no-ops for us: + `f466cfa38` skips a null `dp.result` when `dp.drafting` is false, and the NIF + sets `drafting = true` on every seq immediately before each + `common_speculative_draft`; `2c6b141ef` fixes `draft-mtp` under + `--embeddings` inside `common_base_params_to_speculative`, which the NIF does + not call — it builds both contexts itself. + All three defects in [docs/release-guide.md](docs/release-guide.md) still + stand, re-checked as a source diff: `ggml_backend_rpc_start_server` still + returns `void`, `ggml_backend_cuda_comm_init` was untouched (#26502's + tensor-split work was reverted in `f20395dae`), and the `ggml-cpu` CMake diff + is OpenMP target variables, KleidiAI SME2 GEMV sources and IntelLLVM + fast-math gating — nothing near the `-mcpu=native` probe. +- **The `:mtp_cancel` bug no longer aborts the VM — it returns an error.** The + race is unchanged and unfixed: cancellation is fire-and-forget, so reusing an + `%MTP{}` session immediately after halting a stream can start decoding on + contexts the cancelled loop has not released. What moved is the consequence. + Measured on M1 Max / Metal with Qwen3.6-35B-A3B-MTP, four runs per build: at + b10435 it aborted **4 of 4** (one exit 134 on + `GGML_ASSERT(buf != NULL && "tensor buffer not set")`, three exit 139), at + b10582 it aborted **0 of 4** — three runs failed with + `{:error, "prompt decode failed: code=-1"}` / `"verify decode failed: + code=-1"` and one passed. The decode paths now refuse the half-released + context instead of writing through it. The test stays on its own tag: a flaky + failure still does not belong in a green run, and the real fix is still an + acknowledged-cancellation protocol, not a bump. +- **`--include rpc_live` must run on its own**, documented in + `test/test_helper.exs` after it took the VM down here. The live test calls + `RPC.add_server/1`, which mutates the *process-global* ggml device registry, + and llama.cpp puts RPC devices at the **front** of the placement list built + from that registry — so a model loaded by any *later* test with + `n_gpu_layers: -1` puts layers and KV cache on the worker. Combined with the + model tags, that reached `ggml-rpc.cpp:576` "Remote RPC server crashed or + returned malformed response" inside `llama_kv_cache`'s constructor, with the + worker still listening afterwards: a peer-side allocation failure is + `RPC_STATUS_ASSERT`, which is `GGML_ABORT`, which takes the BEAM with it. ## v0.8.43 diff --git a/Makefile b/Makefile index 44a8a4a..3ffe0f2 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,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 ?= 9e40df63ba151d771d8b247ac4011cf203337e99 +LLAMA_COMMIT ?= e85caa81ea2b65797396018c179b87ad61fa38ab # 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/docs/release-guide.md b/docs/release-guide.md index a7056e0..09c9c85 100644 --- a/docs/release-guide.md +++ b/docs/release-guide.md @@ -96,12 +96,16 @@ upstream fixes, are drafted in `.claude/plans/dgx-spark-2node/upstream-issues.md` — not yet filed, so there are no issue URLs to link. **When they are filed, put the URLs in this table.** -Re-checked at `a94d563ed801` (61 commits later): all three still stand. That -check was a source diff, not a re-measurement — the files each defect lives in -(`ggml/src/ggml-cpu/CMakeLists.txt`, `ggml_backend_cuda_comm_init`, and -`ggml_backend_rpc_start_server`) were untouched by the bump. A source diff is -enough to say a defect is *still there*; it is not enough to say it is *gone*, -so if a diff ever shows movement, run the command in the last column. +Re-checked at `e85caa81e` (b10582): all three still stand. Every re-check so far +has been a source diff, not a re-measurement. At `a94d563ed801` the three files +involved (`ggml/src/ggml-cpu/CMakeLists.txt`, `ggml_backend_cuda_comm_init`, +`ggml_backend_rpc_start_server`) were untouched. At `e85caa81e` the CUDA and RPC +ones are still untouched — #26502 moved the tensor-split meta backend and was +reverted in `f20395dae` — while `ggml-cpu/CMakeLists.txt` did change: OpenMP +target variables, KleidiAI SME2 GEMV sources, and IntelLLVM fast-math gating, +none of it near the `-mcpu=native` probe. A source diff is enough to say a defect +is *still there*; it is not enough to say it is *gone*, so if a diff ever touches +the probe itself, run the command in the last column. | # | Upstream defect | Our workaround | Still needed? | |---|---|---|---| @@ -123,11 +127,38 @@ deliberately do not work around. # the llama.cpp commit, so the bump from step 1 already forces a rebuild. LLAMA_BACKEND=cpu mix compile -# Run full test suite -LLAMA_MODEL_PATH=~/Downloads/Qwen3.5-0.8B-UD-Q4_K_XL.gguf \ -LLAMA_EMBEDDING_MODEL_PATH=~/Downloads/Qwen3-Embedding-0.6B-f16.gguf \ +# Run the suite. The default run needs no model; each opt-in tag names the env +# var for the model it loads (see test/test_helper.exs). GGML_METAL_NO_RESIDENCY +# is Metal-only, and only stops a post-suite assert in llama.cpp's Metal device +# destructor from aborting the VM after a green run. mix test +GGML_METAL_NO_RESIDENCY=1 \ +LLAMA_SMOKE_GEN_MODEL=~/Downloads/Qwen3.5-0.8B-UD-Q4_K_XL.gguf \ +LLAMA_SMOKE_EMB_MODEL=~/Downloads/Qwen3-Embedding-0.6B-f16.gguf \ +LLAMA_SMOKE_MTP_MODEL=~/Downloads/Qwen3.6-35B-A3B-MTP-UD-Q4_K_XL.gguf \ + mix test --include smoke --include embeddings --include slow --include mtp + +GGML_METAL_NO_RESIDENCY=1 \ +LLAMA_SMOKE_MTP_MODEL=~/Downloads/Qwen3.8-27B-Q4_K_M.gguf \ +LLAMA_SMOKE_MTP_DRAFT_MODEL=~/Downloads/mtp-Qwen3.8-27B-Q4_0.gguf \ + mix test --include mtp_sidecar + +# :rpc_live needs an RPC build AND a reachable worker, and must run with no +# model tag beside it — see test/test_helper.exs for why combining them aborts. +# The worker can be local: another BEAM running LlamaCppEx.RPC.Server, or +# `LLAMA_RPC=1 make rpc-server` and upstream's ggml-rpc-server binary. +LLAMA_RPC=1 LLAMA_BACKEND=metal MIX_ENV=test mix run --no-halt -e \ + 'LlamaCppEx.RPC.Server.start_link(endpoint: "127.0.0.1:50052")' & +GGML_METAL_NO_RESIDENCY=1 LLAMA_RPC=1 LLAMA_RPC_ENDPOINT=127.0.0.1:50052 \ + mix test --include rpc_live + +# :mtp_cancel is the one tag that is not expected to pass; run it to confirm +# how it fails, and update test/mtp_model_test.exs if the failure mode moved. +GGML_METAL_NO_RESIDENCY=1 \ +LLAMA_SMOKE_MTP_MODEL=~/Downloads/Qwen3.6-35B-A3B-MTP-UD-Q4_K_XL.gguf \ + mix test --only mtp_cancel + # Verify formatting and types mix format --check-formatted mix dialyzer diff --git a/test/mtp_model_test.exs b/test/mtp_model_test.exs index ac2196a..8daedd9 100644 --- a/test/mtp_model_test.exs +++ b/test/mtp_model_test.exs @@ -150,11 +150,21 @@ end defmodule LlamaCppEx.MTPCancelTest do # KNOWN BUG, kept as an executable record rather than deleted. # - # This test does not fail — it takes the VM down, with - # `GGML_ASSERT(offset + size <= ggml_nbytes(tensor))` or a plain SIGSEGV. So it - # lives in its own module carrying only `:mtp_cancel`, and deliberately *not* - # `:mtp`: `--include` beats `--exclude` in ExUnit, so a second gate tag would - # drag it back into `--include mtp` runs and abort them. Run it on purpose: + # At b10435 this test did not fail — it took the VM down, with + # `GGML_ASSERT(offset + size <= ggml_nbytes(tensor))` or a plain SIGSEGV. + # At b10582 it no longer aborts: it fails, racily, with + # `{:error, "prompt decode failed: code=-1"}` or `"verify decode failed: + # code=-1"` from the `generate/3` below — and sometimes passes. Measured on + # M1 Max / Metal with Qwen3.6-35B-A3B-MTP, four runs each: b10435 aborted 4/4 + # (one exit 134, three exit 139), b10582 aborted 0/4 (3 failed, 1 passed). + # The race is unchanged and unfixed; only its consequence moved from "kills + # the BEAM" to "returns an error", so the two decode paths now refuse the + # half-released context instead of writing through it. + # + # It therefore still lives in its own module carrying only `:mtp_cancel`, and + # deliberately *not* `:mtp`: `--include` beats `--exclude` in ExUnit, so a + # second gate tag would drag a flaky test back into `--include mtp` runs. + # Run it on purpose: # # GGML_METAL_NO_RESIDENCY=1 LLAMA_BACKEND=auto \ # LLAMA_SMOKE_MTP_MODEL=... mix test --include mtp_cancel diff --git a/test/test_helper.exs b/test/test_helper.exs index 95a815a..873a99a 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -10,10 +10,12 @@ # tag rather than `:mtp` because that tag's single-file model # cannot satisfy it. # :mtp_cancel — one known-broken MTP test, excluded on its own tag so that -# `--include mtp` is green. It does not fail, it aborts the VM: -# cancelling an MTP stream is fire-and-forget, so reusing the -# session immediately afterwards races the still-running draft -# loop over shared contexts. See test/mtp_model_test.exs. +# `--include mtp` is green. Cancelling an MTP stream is +# fire-and-forget, so reusing the session immediately afterwards +# races the still-running draft loop over shared contexts. At +# b10435 that aborted the VM; at b10582 it fails racily instead +# (3 of 4 runs) and no longer aborts. Either way it does not +# belong in a green run. See test/mtp_model_test.exs. # :slow — long-running comparison matrices (F16 vs Q8_0 KV cache); # needs LLAMA_SMOKE_GEN_MODEL # :rpc_live — needs a *reachable RPC worker*, not just a model: set @@ -27,6 +29,18 @@ # either refusal used to hide the stale-artifact bug the # Makefile's link marker exists to catch. # +# Run `--include rpc_live` on its OWN, with no model tag beside +# it. The live test calls RPC.add_server/1, which mutates the +# process-global ggml device registry, and llama.cpp puts RPC +# devices at the FRONT of the placement list it builds from that +# registry (src/llama.cpp) — so every model a *later* test loads +# with n_gpu_layers: -1 puts layers and KV cache on the worker. +# Combining the tags aborted the VM here at +# ggml-rpc.cpp:576 "Remote RPC server crashed or returned +# malformed response" during llama_kv_cache construction, with +# the worker still listening: an alloc that fails on a peer is +# RPC_STATUS_ASSERT, which is GGML_ABORT. +# # `--include` beats `--exclude` in ExUnit, so the tags are independent: opt into # exactly the ones whose model you have. The helper `LlamaCppEx.TestModels` # raises with the env var name when an included test has no model to load. diff --git a/vendor/llama.cpp b/vendor/llama.cpp index 9e40df6..e85caa8 160000 --- a/vendor/llama.cpp +++ b/vendor/llama.cpp @@ -1 +1 @@ -Subproject commit 9e40df63ba151d771d8b247ac4011cf203337e99 +Subproject commit e85caa81ea2b65797396018c179b87ad61fa38ab