Repository navigation
fix: align llama-server b10621 compatibility - #1454
Merged
Merged
Conversation
Accept the canonical nightly environment names and safe CLI, request, cache-type, and route aliases while preserving unambiguous legacy fallbacks at lower precedence. Align server sampling defaults, validate cache-reuse semantics without changing unrelated cache behavior, and document the compatibility surface with focused regression coverage for issue #1430.
inureyes
added
status:review
Restore the positive `--slots` override of `_no_slots` in both server entry points so repeated endpoint flags consistently use the last occurrence. Add parser regressions for both argument orders on `mlxcel serve` and `mlxcel-server`. Validated with `cargo test --release --bins slots_flags_use_last_occurrence` and `cargo fmt --check`. Refs #1430
Member
Author
Independent real-checkpoint validationI validated the sampling-default change on Apple GPU (Metal) with
This confirms that the default change is behaviorally effective and reproducible with a real checkpoint rather than merely changing The PR does not modify model-forward, quantized-kernel, or sampler-algorithm code. I nevertheless attempted to build |
29 of 38 tasks
8 tasks done
inureyes
added a commit
that referenced
this pull request
Aug 27, 2026
…uction (#1464) ## Summary Implements llama-server b10621's four RoPE runtime overrides for real, refuses the five YaRN knobs at anything other than upstream's sentinel, and makes the resolved context and batch geometry observable. Everything in this shard that is not implemented now carries a specific, machine-readable divergence instead of a prose note. ## What changed - `src/models/rope_overrides.rs` (new): the pure decision table for `--rope-scaling` / `--rope-scale` / `--rope-freq-scale` / `--rope-freq-base`, plus the process-wide slot, the applications counter, and `verify_applied`. - `src/cli/rope_args.rs` (new): one canonical clap group for the nine b10621 spellings and their `LLAMA_ARG_*` bindings, flattened into both `mlxcel serve` and `mlxcel-server` so the two cannot drift, and `from_env` / `install_from_env` for out-of-band harnesses. - `src/models/rope_utils.rs`, `dynamic_ntk_rope.rs`, `llama3.rs`, `qwen3.rs`, `qwen3_moe.rs`, `apertus.rs`, `gemma3.rs`: the six families on the shared RoPE path consult the override for both the `rope_scaling` block and the base. On Gemma 3 the base override reaches the global-attention layers only; the sliding layers keep `rope_local_base_freq`, which is llama.cpp's separate `rope_freq_base_train_swa`. - `src/server/cli_input.rs`, `startup.rs`, `model_worker.rs`: resolve at the CLI edge (an unserveable request fails the command line, not the first token), install before anything can load a checkpoint, and verify after the load returns. - `src/server/routes/props.rs`: `/props` reports `n_ctx`, `n_batch`, `n_ubatch`, `n_batch_decode` and `n_kv_max`. `n_ctx` is the per-slot window rather than the `--ctx-size` total, matching llama-server and matching what a single request is actually bounded by. - `examples/logit_trace.rs`: installs the override from `LLAMA_ARG_ROPE_*` and records it in the trace header, so the two arms of a comparison are two processes selected the same way the server selects them. - `compat/llama-server/b10621/runtime-and-context.json`: four entries to `supported`; thirteen others get precise `divergence` lists. - `docs/llama-server-compat.md`: an operator-facing section covering the flags, the two refusals, which architectures honor the override and why there is no list of them in the code, and how to validate a change to this path. ## Why the applications counter rather than an architecture list Every model family reads its own `config.json` inside its own `load()`, so there is no argument to thread from the CLI down to the rotation, and only six families route through `models::rope_utils`. An override reaching none of them would leave the server answering every request with the checkpoint's own frequencies, fluently, with nothing to say otherwise, which is exactly the silent acceptance epic #1431 is about. A hand-maintained table of honoring `ModelType`s goes stale the moment a family is ported, so each seam increments a counter instead and the worker refuses to serve when an override was requested and the counter is still zero after the load. A family that wires itself into `rope_utils` starts being accepted with no list to update; one that does not is named in the error alongside the ones that are. ## What is deliberately refused `--rope-scaling yarn` has no representation on a path that builds `default`, `linear` and `llama3` tables only, and the nearest thing (falling back to the unscaled table) is the silent degradation this work exists to prevent. A bare `--rope-scale` or `--rope-freq-scale` on a checkpoint declaring a banded scheme is refused for the same reason: llama.cpp multiplies its `rope_freq_scale` into that rotation, mlxcel's banded table has no such multiplier, and dropping either half changes the result without saying so. The five `--yarn-*` flags are accepted at b10621's sentinels (`-1.0`, `0` for `--yarn-orig-ctx`), which upstream defines as "use the values the model was trained with", so a deployment script spelling out the upstream defaults keeps working; any other value is a startup error naming every offending flag in one message. ## Numerical validation Teacher-forced logit trace per `docs/benchmarks.md`, `mlx-community/Llama-3.2-1B-Instruct-4bit` (`rope_theta` 500000, `rope_scaling` llama3 factor 32), 8192 traced positions across four 2048-wide chunks with 4096 tokens of preceding context, 2464 decided positions at gap >= 2.0. Short prompts are blind to this class of defect, so the shape matters: the traced positions of chunk `c` start at `c * CHUNK_TOKENS`, which is why a narrow chunk cannot reach a long position however large the context argument is. | Arm | top-1 disagreement | on decided positions | true perplexity | |---|---|---|---| | `--rope-freq-base 500000` (== the checkpoint's own) vs no override | 0 / 8192 (0.000%) | 0 / 2464 (0.000%) | 29.5 -> 29.5 | | `--rope-scaling none` vs the checkpoint's llama3 table | 1352 / 8192 (16.504%) | 14 / 2464 (0.568%) | 29.5 -> 32.2 | | `--rope-scaling none` vs `--rope-scaling linear --rope-scale 1` | 0 / 8192 (0.000%) | 0 / 2444 (0.000%) | 32.2 -> 32.2 | | `--rope-freq-base 100000` vs the checkpoint's 500000 | 2496 / 8192 (30.469%) | 131 / 2464 (5.317%) | 29.5 -> 40.7 | | `--rope-scaling linear --rope-scale 32` vs llama3 factor 32 | 7606 / 8192 (92.847%) | 2283 / 2464 (92.654%) | 29.5 -> 3559 | The first row is the control that matters most: requesting the base the checkpoint already declares must come back byte-identical, which separates "the override moved the rotation" from "the override plumbing perturbs the model". The third row shows two spellings of one request agreeing byte for byte. The fifth shows `linear` and `llama3` at the same factor are emphatically not the same rotation, which is the conflation a naive implementation would make. Every override degrades long-context quality against a correctly configured checkpoint, which is the expected direction. Note that `scripts/compare_logit_traces.py` prints `exp(mean log-prob)`, the reciprocal of perplexity, so its negative percentages are quality regressions; the table above states true perplexity. ## Scope this issue does not close `--keep`, `--swa-full`, `--context-shift`, `field:n_keep` and `field:n_discard` stay `deferred`, now with their observable divergences recorded rather than described in prose: the retained prefix is still the fixed 4-token attention sink in `src/server/batch/scheduler.rs`, KV front-trimming is unconditional once a context bound exists and cannot be disabled (b10621 defaults context shift off and fails an over-long request), and sliding-window families build their own `RotatingKVCache` inside each model's `make_cache` with no server-side reach into it. `--parallel` records its two real divergences (default 4 against b10621's -1, and -1 rejected by the value parser); the rejection behavior was deliberately established in #1454 one commit ago, so this PR records it rather than reversing it. `--warmup` and `--escape`, which the issue's scope mentions, live in `model-source.json` (#1434) and `chat-templates.json` (#1447), shards this chain does not own; the manifest gate would reject an edit to either. ## Test plan - [x] `cargo test --profile test-fast --features metal,accelerate --lib models::rope` (37 passed) - [x] `cargo test --profile test-fast --features metal,accelerate --lib server::cli_input` (98 passed), `--lib server::startup` (61), `--lib server::llama_compat` (3), `--lib server::routes::props` (8) - [x] `cargo test --profile test-fast --features metal,accelerate --test llama_compat_manifest` (4 passed, including the archive-gated conformance test run against the verified b10621 macOS arm64 archive via `MLXCEL_LLAMA_B10621_DIR`) - [x] `cargo test --profile test-fast --features metal,accelerate --test llama_compat_rope_scaling_override` and `--test llama_compat_rope_freq_base` (1 each; one configuration per process, because the override is a `OnceLock`) - [x] `cargo test --profile test-fast --features metal,accelerate --test cli_help_consistency` (25 passed) - [x] `python3 scripts/ci/check_llama_compat_manifest.py` and `bash scripts/ci/check_llama_compat_manifest_test.sh` - [x] `cargo clippy --profile test-fast --features metal,accelerate --lib --bins --tests -- -D warnings`, `cargo fmt --all -- --check`, `python3 scripts/ci/check_cross_repo_refs.py` - [x] Numerical validation on a real checkpoint, as tabulated above Closes #1450
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.
Summary
LLAMA_ARG_CACHE_REUSEas an integer reuse-chunk setting:0is a no-op, while booleans and unsupported positive values fail startup with actionable errors.top_p,min_p, anddry_penalty_last_n, including the/propsconfiguration source.seed: -1as nondeterministic sampling; values below-1are rejected./v1/health,/reranking, and/v1/reranking, keep every health alias public under API-key authentication, and accept--temperatureon both binaries.f16split-cache alias for FP16 while continuing to reject GGML quantizer names such asq8_0andq4_0rather than translating them to different arithmetic.Scope
This PR maps
repeat_last_nto the pre-existing OpenAI-shaped request field but intentionally does not change sampler arithmetic; runtime penalty-window parity and its numerical validation remain owned by #1436.Validation
cargo test --release server::types::request::tests::— 25 passed.cargo test --release --bin mlxcel serve_canonical_llama_envs_and_endpoint_precedence_parse_together— 1 passed.cargo test --release --bin mlxcel-server canonical_llama_envs_and_endpoint_precedence_parse_together— 1 passed.cargo test --release --lib cli::turbo_args::tests::split_cache_flags— 2 passed.cargo test --release --lib prompt_cache_llama_arg_cache_reuse— 3 passed.cargo test --release --lib prompt_cache_mlxcel_env_does_not_hide_invalid_cache_reuse— 1 passed.cargo test --release --lib server_default_env_fallback_all_cases— 1 passed.cargo test --release --lib server_config_default_matches_llama_server_compatibility_defaults— 1 passed.cargo test --release --lib build_server_config_prompt_cache_default_is_enabled— 1 passed.cargo test --release --lib all_rerank_aliases_are_mounted— 1 passed.cargo test --release --lib v1_health_alias_is_public_when_api_key_auth_is_enabled— 1 passed.cargo test --release --lib all_health_aliases_are_unauthenticated— 1 passed.cargo test --release --bin mlxcel serve_temp_and_temperature_aliases_resolve_identically— 1 passed.cargo test --release --bin mlxcel-server temp_and_temperature_aliases_resolve_identically— 1 passed.cargo test --release --test cli_help_consistency turbo_flags_and_modes— 4 passed.cargo fmt --check— passed.Closes #1430