Repository navigation
feat(inference): complete b10621 sampling semantics and windows - #1487
Merged
Merged
Conversation
Close the b10621 sampling-semantics gaps that #1436 owns in the compatibility manifest, and split the remainder to #1485. Penalty window: `SamplingConfig::penalty_last_n` (b10621 `repeat_last_n`) now reaches the sampler. `-1` (the default, and the CLI default) scans the full history exactly as before, `0` disables the repetition/frequency/presence stage entirely (the config then stays fused-batch eligible via `needs_token_history`), and `N > 0` penalizes over only the last N tokens. The incremental `SamplerState` keeps serving the full-history form byte-identically; a positive window takes the bounded rebuild path. The server resolves the request `repetition_context_size` (alias `repeat_last_n`, previously parse-only) or falls back to the server-wide `--repeat-last-n` default of 64, matching b10621; this changes penalized server-side generations that omit the field, which previously scanned the whole history. DRY sentinels: `dry_penalty_last_n: 0` now DISABLES DRY (b10621's sentinel) and the pre-existing "0 = full history" form moved to the explicit `DRY_FULL_HISTORY` (spelled `-1` on the CLI flags); the penalty comparison changed from `>` to `>=` so a repeat of exactly `dry_allowed_length` receives the `base^0` tier; a `dry_base` below 1.0 falls back to the server default on the HTTP path and keeps the CLI default silently, both matching upstream. The mlxcel-xla parity sampler mirrors all of it. New server surfaces on both binaries (`mlxcel serve`, `mlxcel-server`): `--top-nsigma` / `--top-n-sigma` (default -1.0, the b10621 disabled sentinel, sanitized to the 0.0 disabled form), `--xtc-probability`, `--xtc-threshold`, `--ignore-eos`, repeatable `-r` / `--reverse-prompt` (server-wide stop strings merged with request stops), and `--samplers` / `--sampler-seq` / `--sampling-seq` which accept exactly b10621's default chain order (the inert configuration, since #1375/#1377 pin the filter positions to it) and fail startup with an actionable diagnostic otherwise. `/props` reports the new defaults. Native /completion: `top_n_sigma` (<= 0 maps to the explicit disabled form), `xtc_probability` / `xtc_threshold` (b10621's 0..=1 schema limits enforced with a 400, an inert threshold above 0.5 accepted), `ignore_eos` (implemented as upstream does, a -inf logit bias on the merged end-of-generation set at enqueue time), `repeat_last_n` wiring, and `samplers` (default order accepted in both upstream shapes, any other order refused with a 400 naming the accepted forms). Manifest: 26 entries flip to `supported`, 3 to `not_applicable` with tested rejection, and 31 entries this issue could not honestly claim (Mirostat, dynatemp, adaptive-p, grammar surfaces, logit-bias, dry-sequence-breaker strings, min_keep, n_probs, post_sampling_probs, backend_sampling, the temp/top-k/top-p generation_config default divergence, and the seed below-minus-one divergence) repoint to the new #1485, which pin.json now lists as a shard co-owner. The `--dry-base` divergence records are cleared because the behavior now matches. Validation: 98 core sampling tests including window sentinels, window-vs-full byte identity when the window covers the history, zero-window inertness plus fused eligibility, DRY 0-disable, full-history sentinel, and the >= tier; xla parity suite; compat integration against both rebuilt binaries; real-checkpoint arms on Qwen3-4B-4bit with greedy decoding (every position decided): full-history sentinel vs covering window byte-identical, window-present-penalties-off byte-identical to baseline, DRY 0-sentinel byte-identical to DRY-off, and the window-64 treatment arm diverging from full history first at word 76 of 164 (53 percent of subsequent words) exactly as a forgetting window should. The traced forward path is untouched by this change; every edit is post-forward sampling arithmetic and plumbing. Refs #1431.
Review findings against the pinned b10621 source, each one a place where the first commit's manifest claim described upstream wrongly: - Stop strings: upstream seeds the task's stop set from the CLI antiprompt list and REPLACES it wholesale when the request carries any effective stop string, falling back to the CLI list only when the request supplied none; the resolution now does exactly that instead of concatenating, and the test asserts the replace semantics. - XTC domains: b10621 declares SOFT 0..=1 schema limits on both XTC fields, clamping out-of-range values instead of rejecting them; the native route now clamps identically (no more 400), with the struct docs, manifest notes, and tests updated. - DRY flag domain: b10621's CLI rejects a negative --dry-penalty-last-n outright, so both server binaries now reject negatives at parse time (the resolve fn keeps its negative arm as defense in depth); a full-history scan stays reachable through a window at least the context length, and the offline generate CLI keeps its -1 spelling as a documented mlxcel-only extension. - dry_base: the startup value is sanitized like typical_p (a sub-1.0 flag folds to the 1.75 default with a warning, matching upstream's silent keep), and both the core and xla DRY implementations mirror upstream's below-1.0 early-out so the request-layer sanitization has a backstop. The DRY exponent now carries upstream's FLOAT_MAX_LOG / ln(base) cap so a very long full-history match cannot push the penalty to infinity, and the shared DEFAULT_DRY_BASE const keeps the CLI fallback and the clap default from drifting. - ignore_eos scope: the OpenXLA admission path rejects an ignore_eos request with a diagnostic (it has no token-bias path) instead of silently stopping at EOS, the disaggregated handoff records the same gap beside the existing XTC note, and both manifest entries carry the scope qualifier. - Hygiene: HTTP dry_penalty_last_n clamps to upstream's INT32_MAX ceiling so a wire value can never collide with the internal DRY_FULL_HISTORY sentinel; the lossy repetition-window cast uses a checked conversion; allow_hyphen_values became allow_negative_numbers on every new flag per the documented parser policy; the check_sampler_order call moved ahead of the multi-GB model load (verified failing in 21 ms); the preprocess_logits_for_sampling and resolve_default_max_tokens doc blocks that had been hijacked by inserted helpers were restored to their functions; the bench literal spells DRY_FULL_HISTORY. Refs #1436.
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
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
Completes the b10621 sampling semantics that issue #1436 owns in the compatibility manifest: the
repeat_last_npenalty window finally reaches the sampler, the DRY sentinels and off-by-one tier are reconciled with upstream, the top-n-sigma / XTC / ignore-eos / reverse-prompt server and native surfaces land on both binaries, and the sampler-order flags accept exactly the fixed b10621 default chain. 26 manifest entries flip tosupported, 3 tonot_applicablewith tested rejection, and the 31 entries this PR cannot honestly claim repoint to the new remainder issue #1485, added as a shard co-owner inpin.json.Behavior changes to know about
repetition_context_size(aliasrepeat_last_n) are now bounded to the server-wide--repeat-last-ndefault of 64 recent tokens, matching b10621; before this PR the flag was parsed and echoed by/propsbut every penalty scanned the whole history. The CLI keeps its pre-existing full-history behavior via a-1default on the new--repeat-last-nflag.dry_penalty_last_n: 0now disables DRY (upstream's sentinel); the pre-existing "0 = whole history" form moved to the explicitDRY_FULL_HISTORY, spelled-1on the CLI flags. The DRY comparison changed from>to>=, adding upstream'sbase^0penalty tier at exactlydry_allowed_length. Adry_basebelow1.0falls back to the server default on HTTP and keeps the CLI default silently, both matching b10621.penalty_last_n == 0makes the penalty stage inert regardless of penalty values, so such configs stay on the fused batch path (needs_token_historygates on the windows now).Rust versus C++ placement
No C++ or bridge change. The penalty window slices the token history handed to the existing Rust penalty functions ahead of the fused dispatch; the incremental
SamplerStatecontinues to serve only the full-history form (byte-identical, proven by the covering-window arm below), and a positive window takes the bounded rebuild path over at mostrepeat_last_ntokens.ignore_eosreuses the shared token-bias map (a-infbias on the merged end-of-generation set at enqueue time), exactly upstream's EOG-bias mechanism.What changed
src/lib/mlxcel-core/src/generate.rs+sampling.rs:SamplingConfig::penalty_last_n,DRY_FULL_HISTORY,penalty_window, windowed penalty application, DRY sentinel arms and>=tier,needs_token_historywindow gating.src/lib/mlxcel-xla/src/sampler.rs+src/server/batch/xla_worker_admission.rs: the OpenXLA parity sampler mirrors the same window and DRY semantics (note for reviewers: this PR adds fields consumed byxla-iree-gated code; theOpenXLA feature compilejob is the gate that sees those arms, though the xla crate itself compiles and passes its suite locally).--top-nsigma/--top-n-sigma(default-1.0, sanitized),--xtc-probability,--xtc-threshold,--ignore-eos, repeatable-r/--reverse-prompt(merged into every request's stop set),--samplers/--sampler-seq/--sampling-seq(fixed-order validation failing startup with the accepted form in the diagnostic), plus--repeat-last-nsemantics and--dry-penalty-last-nsentinel mapping;/propsreports the new defaults with the key-set contract updated./completion:top_n_sigma,xtc_probability/xtc_threshold(schema limits enforced with a 400),ignore_eos,repeat_last_nwiring, andsamplers(default order accepted in both upstream shapes, others refused with a 400);generation_settingsechoes the new resolved values.SerializableSamplingState::penalty_last_nwith a-1serde default so an older peer's frame keeps its full-history penalties; round-trip and legacy-frame tests. Note: an old frame carryingdry_penalty_last_n: 0(the old full-history spelling) is read as disabled by a new decode node during a mixed-version handoff window; recorded here rather than silently ignored.compat/llama-server/b10621/sampling-and-grammar.json(27 flips, 31 repoints, two now-fixed--dry-basedivergence records cleared,field:seedkept deferred under feat(inference): complete the remaining b10621 sampling and grammar surfaces #1485 because its below-minus-one rejection divergence is real) andcompat/llama-server/b10621/pin.json(adds feat(inference): complete the remaining b10621 sampling and grammar surfaces #1485 to the shard owner set, the mechanism the validator itself prescribes for splitting a remainder).docs/python-client.md,docs/llama-server-compat.mdowner table.Numerical validation (Qwen3-4B-4bit, greedy decoding, every position decided)
--repeat-last-n -1(full-history sentinel) vs--repeat-last-n 100000(window covering the whole run), repetition 1.3, 200 tokens--repeat-last-n 64with all penalties at identity vs no flags--dry-multiplier 0.8 --dry-penalty-last-n 0vs DRY off--repeat-last-n 64vs full history, repetition 1.3, 200 tokensbase^0tierThe teacher-forced forward path is untouched: no model or kernel file changes, so the logit-trace forward comparison is identically zero by construction; every change is post-forward sampling arithmetic, validated by the fixed-logit unit tests and the greedy real-checkpoint arms above.
Review follow-up (second commit)
A review pass against the pinned b10621 source corrected three claims the first commit got wrong about upstream, plus hardening: request stop strings now REPLACE the server-wide
--reverse-promptlist when non-empty (upstream's antiprompt fallback semantics) instead of merging; the native XTC fields now CLAMP into b10621's soft 0..=1 limits instead of answering 400; both server binaries reject a negative--dry-penalty-last-nat parse time exactly as b10621's CLI does (the offline generate CLI keeps-1as a documented mlxcel-only spelling);--dry-baseis sanitized at startup and the core plus xla DRY implementations mirror upstream's below-1.0 early-out and exponent cap; the OpenXLA admission path rejectsignore_eoswith a diagnostic instead of silently stopping at EOS and the disaggregated gap is recorded beside the existing XTC note; HTTPdry_penalty_last_nclamps to INT32_MAX so a wire value cannot collide with the internal full-history sentinel;allow_hyphen_valuesbecameallow_negative_numbersper the documented parser policy;check_sampler_orderruns before the model load (fails in 21 ms); and two hijacked doc blocks were restored to their functions. The corresponding manifest notes were rewritten to describe upstream accurately.Test plan
cargo test -p mlxcel-core --profile test-fast --features metal,accelerate --lib sampling::tests(98 passed, including the seven new window/DRY tests)cargo test -p mlxcel-xla --profile test-fast --lib(280 passed; the breaker test now isolates the breaker from the new>=tier)penalty,dry,sampler_order,sanitize,request_options,native_completion,serving_protocol,props,validate): all greencargo test --profile test-fast --features metal,accelerate --test llama_compat_manifest(4 passed against both rebuilt binaries) and thellama_compatlib suitepython3 scripts/ci/check_llama_compat_manifest.py(zero feat(inference): complete b10621 sampling and grammar semantics #1436 deferred entries remain, so closing this issue cannot redden the validator),check_cross_repo_refs.pycargo clippy(-D warnings) on mlxcel-core, mlxcel-xla, and the root lib/tests/bins;cargo fmt --all -- --checkCloses #1436