Fix option forwarding - #77
Merged
Merged
Conversation
Updates vendor/llama.cpp from 76f46ad29 (b10075, released in v0.8.38) to ff067f76d (b10133), 58 commits. Unlike recent bumps this range breaks the upstream C API: llama_model_params loses the use_mmap/use_direct_io/use_mlock booleans in favour of a single `enum llama_load_mode load_mode` field (upstream #20834). model_load/10 now maps the existing :use_mmap / :use_mlock / :use_direct_io options onto that enum, preserving the documented precedence (dio > mlock > mmap > none). The public Elixir API and its defaults are unchanged, so callers need no updates; all four resolved modes were verified against a real model load. common/chat.h also renamed common_chat_params::thinking_end_tag to thinking_end_tags (now a vector, #25544), but the binding never read that field. Full suite against the rebuilt NIF with real GGUF models, including the smoke, slow and MTP speculative-decoding tests: 252 passed, 0 failures. MTP was exercised against a Qwen3.5-9B-MTP GGUF (arch qwen35, nextn tensors present).
The key lists that forward user options into Context.create/2,
Sampler.create/2 and Model.load/2 were hand-maintained copies, and they
had drifted. `mix credo --strict` cannot see this.
Context's list existed in three copies (llama_cpp_ex.ex, mtp.ex, and an
inline literal in server.ex) and the server copy was missing :n_threads,
:n_threads_batch and :n_ubatch. Those keys are read nowhere else in
server.ex, so `Server.start_link(n_threads: 8)` silently ignored them
while `LlamaCppEx.generate(n_threads: 8)` honored them. Server's model
list likewise omitted :use_mmap, leaving Server users unable to select
llama.cpp's `none` load mode. The sampler list turned out to have three
copies too, the third an inline literal in MTP.stream_events/3.
Each consuming module now owns its own list, split by kind:
* tuning keys — safe for any caller to forward (threads, batch sizes,
KV types, flash-attn, RoPE/YaRN, no_perf, swa_full).
* structural keys — set explicitly by each caller, never forwarded
blindly (:n_ctx, :n_seq_max, :kv_unified, :embeddings, :pooling_type,
:ctx_type, :n_rs_seq).
Keeping :embeddings/:pooling_type/:ctx_type structural is the point: it
lets Server's list widen to fix the dropped keys without letting anyone
pass embeddings: true into a generation server. Callers already prepend
their own values (`[n_ctx: computed] ++ forwarded`), which win because
Keyword.get/3 returns the first match, so nothing they compute can be
overridden.
test/option_forwarding_test.exs is the drift alarm: it scans the
option-reading function bodies and fails if a new option is not
classified, if a declared key is never read, or if a caller re-inlines a
literal list. Verified it fails as intended by adding a bogus key.
Full suite with real GGUF models (smoke, slow and MTP included):
263 passed, 0 failures. End-to-end, Server now honors n_ubatch: 64 and
use_mmap: false (load_mode = none); n_threads is not logged by llama.cpp
so it is covered by the same code path and the guard test rather than
observed directly.
Both prompts in "session affinity keeps interleaved conversations on their slots" started with "Chat ". pick_cached_slot/2 reuses a slot when longest-common-prefix / prompt_len > 0.1, and on Llama-3.2's tokenizer that shared opener clears 10% on such short prompts, so chat B was routed onto chat A's slot and `assert slot_a != slot_b` failed. The same test passed on Qwen3.5, so whether the suite was green depended on which model LLAMA_SMOKE_GEN_MODEL pointed at. Give the two prompts disjoint leading tokens, and note the coupling so the shared prefix is not reintroduced. The test is :smoke-tagged and excluded by default, so CI never ran it and the breakage was invisible. Verified with both models: 263 passed, 0 failures each. This also removes a secondary GGML_ASSERT([rsets->data count] == 0) abort (exit 134) that fired only because the failing test left Metal resources alive at exit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.