Conversation
Upstream took the whole ternary engine (b5b892e and friends, mine) but not the HTTP wiring that reaches it: `larql_inference::ternary` exports `predict_bitnet`, `infer_bitnet_walk` and `generate_streaming_bitnet`, and before this commit nothing in larql-server called any of them -- only larql-cli did. The engine landed; the server could not serve it. The wiring existed on feat/bitnet-streaming-walk and was then lost: merge 918a397 on the fork's main dropped `is_bitnet()` / `get_or_load_bitnet()` from state.rs while keeping two callers in routes/openai/. That branch has not compiled since (E0599); f11d8a1 fixed the E0428s from the same merge and missed this. Ported here onto current upstream rather than merged, since state.rs and the openai routes have both since become modules. state/loaded_model.rs `bitnet_model: OnceLock<RwLock<BitnetModel>>` + `bitnet_init` guard, mirroring the existing `weights` / `weights_init` single-flight pattern; `is_bitnet()`, `is_dense_only()`, `get_or_load_bitnet()`, `force_load_bitnet_model()`. bootstrap/mod.rs Eager-load the ternary path instead of the dense one for BitNet containers (~5 GB of dense allocation saved on a 2 B BitNet), and exclude them from the startup memcheck -- `estimate_resident_bytes` models the dense path and over-counts a container that allocates no dense BitLinear tensors. routes/infer.rs Native-ternary /v1/infer, checked *before* the `has_model_weights` gate: a --keep-quant container carries ternary artifacts instead of the dense manifest that gate looks for, so it would otherwise be refused as weightless. Walk-mode goes through residual capture + KNN override, following upstream's current session-resolution idiom (`sessions.get(sid).and_then(|s| s.patched())`, reader not writer). routes/openai/{completions,chat/stream}.rs SSE streaming for both OpenAI surfaces via the ternary path, which also skips the dense `lock_weights_for_gen()` write lock that serialises all generation. Chat refuses tools / constrained generation rather than ignoring them: both need masked logits over the dense path, and answering a tool request with prose looks like a model that declined to call the tool. Adapted to upstream rather than copied: * chat streaming uses upstream's `TokenTap` for stop-string handling instead of the branch's hand-rolled buffering, so both paths share one implementation rather than two that must agree; * `pick_template` now requires `&ModelWeights`, which this path deliberately never loads -- the template comes from `ChatTemplate::for_family(&config.family)`, the same string `weights.arch.family()` would have produced; * both branches record a `GenerationTally` (`add_v3`), else /v1/stats reports BitNet traffic as zero throughput; * `FINISH_REASON_*` constants, not the branch's literal "stop"/"length"; * no inline SSE_DONE -- the response stream already chains it, so the branch's version would have emitted it twice. Also `is_dense_only()`: a --dense-only container has no gate vectors, so walk-mode runs against an empty KNN store and returns nothing useful. /v1/infer defaults mode to walk when a client omits it (pg_infer's remote backend posts {prompt, top} with no mode), which silently produced garbage. Walk/compare now coerce to dense on such containers. Verified with the pinned 1.98.0, not the ambient nix toolchain: clippy -p larql-server --all-targets -- -D warnings: clean (--all-targets matters -- 7 LoadedModel literals in tests/ need the two new fields and `cargo build` alone does not see them) cargo test -p larql-server --no-fail-fast: 1123 passed, 0 failed + 2 new tests (is_dense_only_detects_empty_gate_layers, bitnet_model_not_loaded_by_default) 16 test *binaries* SIGSEGV under --no-fail-fast. Pre-existing and not from this change: the identical 16 crash on unmodified 23a56db (verified by stashing). Untouched here.
|
Heads-up on the two red checks (
This PR's So any PR opened against |
CI's coverage policy flagged the four files the BitNet serving commit
touched. Two responses, split by what is actually testable.
Testable, so tested — state/loaded_model.rs gains two tests beside the
two already there:
* bitnet_guards_refuse_a_dense_vindex_with_a_useful_message —
`get_or_load_bitnet()` on a dense container must name *why* it
refused ("no bitnet_layout ... not a --keep-quant build") rather
than surfacing a load error for a file that was never going to
exist.
* force_load_bitnet_model_is_a_noop_when_infer_disabled —
`bootstrap::serve` calls this unconditionally for every model, so it
has to stay quiet on `--no-infer` even when the container *is*
BitNet-shaped. Eagerly loading ternary weights into a process that
refuses to infer would spend exactly the memory a --no-infer
operator asked not to spend.
Not testable yet, so baselined — routes/infer.rs (75.0),
routes/openai/completions.rs (79.5), routes/openai/chat/stream.rs (66.0),
at the values CI measured. Every ternary arm sits behind
`LoadedModel::is_bitnet()`, which needs a container carrying
`bitnet_layout` plus the `bitnet/` I2_S artifacts, and `synthetic_vindex`
builds a dense V2 container. So those ~300 lines are structurally
unreachable from the fixtures that exist, in the same way this policy
already documents for the V2 per-token emit closures and the tool-success
path.
The real fix is a synthetic BitNet fixture, and it is deliberately not
attempted here: it needs packed I2_S bytes plus per-row scales in the
kernel's contiguous layout, not just a config flag, so it is a piece of
work in its own right rather than a line in this commit. The policy note
records that, so the baselines read as debt with a named discharge
condition instead of as a lowered bar. Ratchet them when the fixture
lands.
`loaded_model.rs` deliberately gets no baseline: the four guard tests
should carry it over the 90% default, and if they do not, that is a real
gap worth seeing rather than papering over.
fmt, clippy --all-targets -D warnings, and 595 lib tests pass on the
pinned 1.98.0. Local full-suite coverage is not measurable on this
machine — its integration binaries SIGSEGV on unmodified main too — so
the baselines are CI's numbers, not mine.
…-> 90) The previous commit's two tests moved loaded_model.rs from 88.63% to 88.99%, 1.01 short of the 90% default floor. This covers the remaining reachable branch rather than adding a baseline for it. bitnet_load_failure_names_the_container: a container that *claims* to be BitNet (bitnet_layout present) but has no bitnet/ artifacts on disk must fail with the load error, not the "not a --keep-quant build" refusal. Those are different operator problems -- the first means "this vindex is the wrong kind", the second means "this vindex is the right kind and is incomplete" -- and reporting the wrong one sends someone to rebuild a container that only needs its files restored. Reachable with no weights: the fixture's path points at no bitnet/ directory, which is exactly the on-disk state of a truncated or partially-copied container. That drives `load_bitnet_model` far enough to return its error, which was the last uncovered branch in `ensure_bitnet_cell` short of a real ternary load. Also asserts a failed load leaves the cell empty, so a later attempt -- after the operator restores the files -- tries again rather than caching the failure for the process lifetime. That is a property of the OnceLock-set-after-success ordering worth pinning, not just a coverage line. fmt, clippy --all-targets -D warnings, 596 lib tests pass.
Found by verifying against the real microsoft/bitnet-b1.58-2B-4T, which
is the only thing that could have found it: the synthetic fixture is a
dense V2 container and *has* the weight files whose absence this is about.
The three non-streaming generation paths — the `/v1/completions` batch
loop, `chat/handler.rs`, and `responses/engine.rs` — all take a
`&mut ModelWeights` for the duration of generation, so they call
`lock_weights_for_gen()`. On a BitNet `--keep-quant` container there are
no dense weights to lock, so `ensure_weights_cell` reached for a manifest
that does not exist and the request came back as:
503 "failed to load model weights: IO error: No such file or
directory (os error 2)"
which tells an operator nothing about the actual situation: the model is
loaded and working, just not through that path.
Guarded in `lock_weights_for_gen()` rather than at the three call sites.
Every non-streaming path funnels through this one method, so one check
covers all of them instead of three that have to stay in agreement — and
the streaming paths are unaffected because they test `is_bitnet()` and
return before they ever reach the lock (completions.rs:381 before :461,
chat/stream.rs:56 before :148).
Refused rather than silently rerouted to the ternary engine: these callers
hold a `&mut ModelWeights` across generation and there is no dense
`ModelWeights` to hand them. The message names the paths that do work
(`POST /v1/infer`, or either OpenAI surface with `"stream": true`), since
the capability exists and only the route is wrong.
Real-model verification, microsoft/bitnet-b1.58-2B-4T (1.2 GB I2_S GGUF
-> `--keep-quant --dense-only --f16 --level inference`, 210 I2_S tensors,
30 layers, hidden 2560):
/v1/infer, no `mode` field -> Paris 0.9494, mode=bitnet
/v1/infer, mode=dense -> Paris 0.9494, mode=bitnet
/v1/infer, mode=walk -> Paris 0.9494, mode=bitnet (coerced)
All three agree to 4dp, which is the point: `is_dense_only()` coerces
walk to dense rather than answering from an empty KNN store. 0.9494
matches the 94.5% the original work measured on the June tree.
Also verified end to end on the real model: eager ternary pre-load
("Pre-loaded BitNet model for 'bitnet2b' in 3.3s" — the ternary path, not
the dense one); `/v1/completions` and `/v1/chat/completions` SSE both
stream coherent text with exactly one `[DONE]` (the duplicate I removed
during the port stayed removed) and `finish_reason: length`; chat refuses
tools with the intended message; `/v1/runtime` reports
`decode_tokens_per_second: 0.98`, i.e. the GenerationTally added during
the port is reaching the stats surface instead of reporting zero.
Throughput on 32 vCPU x86_64, A/B alternated, 3 reps, medians:
infer_short 4.757s (spread 0.008)
infer_long 24.785s (spread 0.100)
gen_8tok 11.629s (0.69 tok/s)
gen_32tok 32.661s (0.98 tok/s)
~1 tok/s is expected here rather than a regression: `ternary_matvec` has
a NEON path under `cfg(target_arch = "aarch64")` and no x86 SIMD
equivalent, so x86_64 runs the scalar kernel. An AVX2/AVX-512 ternary
kernel is the obvious follow-up and is not attempted here.
clippy -p larql-server --all-targets -- -D warnings: clean.
cargo test -p larql-server --no-fail-fast: 1283 passed, 0 failed, 0
crashes (+1 test: lock_weights_for_gen_refuses_bitnet_with_an_actionable_message).
Re-verified against the real
|
| request | result |
|---|---|
/v1/infer, no mode field |
Paris 0.9494, mode=bitnet |
/v1/infer, mode=dense |
Paris 0.9494, mode=bitnet |
/v1/infer, mode=walk |
Paris 0.9494, mode=bitnet (coerced) |
All three agree to 4dp — which is the point of is_dense_only(): walk coerces to dense instead of answering from an empty KNN store. 0.9494 matches the 94.5% measured on the June tree.
Also confirmed on the real model:
- eager ternary pre-load:
Pre-loaded BitNet model for 'bitnet2b' in 3.3s(ternary path, not dense) - both SSE surfaces stream coherent text (
" Paris. Paris is a city that"), exactly one[DONE],finish_reason: length - chat refuses tools with the intended message rather than answering with prose
/v1/runtimereportsdecode_tokens_per_second: 0.98— theGenerationTallyadded during the port reaches the stats surface instead of reporting zero
Throughput (32 vCPU x86_64, A/B alternated, 3 reps, medians)
| case | median | spread | effective |
|---|---|---|---|
infer_short |
4.757s | 0.008s | — |
infer_long |
24.785s | 0.100s | — |
gen_8tok |
11.629s | 2.690s | 0.69 tok/s |
gen_32tok |
32.661s | 0.773s | 0.98 tok/s |
~1 tok/s is expected here, not a regression: ternary_matvec has a NEON path under cfg(target_arch = "aarch64") and no x86 SIMD equivalent, so x86_64 runs the scalar kernel. An AVX2/AVX-512 ternary kernel is the obvious follow-up — happy to open a separate issue for it.
Suite
clippy -p larql-server --all-targets -- -D warnings clean; cargo test -p larql-server --no-fail-fast 1283 passed, 0 failed, 0 crashes.
|
Follow-up on the two red checks: I've opened #481, which bumps On that branch |
CI caught this; my local runs did not. `ingestion_closure.rs` walks every source file and asserts the set of `record`-family call sites matches `ingestion_record_sites.json` exactly — a deliberate ledger, so a new recording route cannot appear without someone naming its owner. The two new sites are the `GenerationTally` recordings in the BitNet ternary arms of `stream_chat_completion` and `stream_completions`, added by 964166b's parent work so `/v1/stats` would not report BitNet traffic as zero throughput. Both owners were already in the ledger with one `record` each; the ternary arm gives each a second. Not a defect in this branch's perf work — the calls predate it. It surfaced here because `ingestion_closure` is a larql-vindex test and its workflow is path-filtered: the branch that introduced the calls (chrishayuk#480) touches only larql-server, so the test never ran there and chrishayuk#480 shows 16/16 green. This branch touches larql-vindex, so it ran. Worth noting for the reviewer of chrishayuk#480: that PR is green for a path-filter reason, not because the ledger agrees with it. Insertion only — the file stays sorted by (file, owner, call) and no existing entry moved (diff is +10 lines, nothing removed). cargo test -p larql-vindex --test ingestion_closure: 2 passed. clippy --all-targets -- -D warnings: clean. fmt: clean. (The lib tests SIGSEGV intermittently on my local box — a known artifact of that machine, not this change; CI runs the same 4736 tests green on ubuntu, macos and windows.)
The ternary engine landed here in #159 / b5b892e, but the HTTP wiring that reaches it did not.
larql_inference::ternaryexportspredict_bitnet,infer_bitnet_walkandgenerate_streaming_bitnet, and today nothing inlarql-servercalls any of them:Only
larql-cli(convert_cmd.rs,run_cmd.rs) does. So a--keep-quantcontainer can be built and run from the CLI, butlarql-servercannot serve it — the engine is there and unreachable over HTTP.This adds that wiring.
What it does
state/loaded_model.rs—bitnet_model: OnceLock<RwLock<BitnetModel>>plus abitnet_initguard, mirroring the existingweights/weights_initsingle-flight pattern exactly.is_bitnet(),is_dense_only(),get_or_load_bitnet(),force_load_bitnet_model().bootstrap/mod.rs— eager-loads the ternary path instead of the dense one for BitNet containers (~5 GB of dense allocation saved on a 2B BitNet), and excludes them from the startup memcheck:estimate_resident_bytes()models the dense path and would massively over-count a container that allocates no dense BitLinear tensors.routes/infer.rs— native-ternary/v1/infer, checked before thehas_model_weightsgate. That ordering is load-bearing: a--keep-quantcontainer carries ternary artifacts rather than the dense weight manifest that gate looks for, so it would otherwise be refused as weightless. Walk-mode goes through residual capture + KNN override.routes/openai/{completions,chat/stream}.rs— SSE streaming for both OpenAI surfaces via the ternary path, which also skips the denselock_weights_for_gen()write lock that serialises all generation. Chat refuses tools and constrained generation rather than ignoring them: both need masked logits over the dense path, and answering a tool request with prose looks like a model that declined to call the tool.is_dense_only()A
--dense-onlycontainer has the dense weights and I2_S artifacts but no gate vectors, so walk-mode runs against an empty KNN store and returns nothing useful./v1/inferdefaultsmodetowalkwhen a client omits it — which any client posting{prompt, top}does — so this silently produced garbage on a correct model. Walk and compare now coerce to dense on such containers.Adapted to current main, not replayed
This was originally written against a June tree;
state.rsand the openai routes have both since become modules, so it is ported rather than merged. Four things changed in the process, each of which would have been a defect if copied verbatim:TokenTapfor stop-string handling instead of the original hand-rolled buffering, so both paths share one implementation rather than two that must agree;pick_templatenow takes&ModelWeights, which this path deliberately never loads — the template comes fromChatTemplate::for_family(&config.family), the same stringweights.arch.family()would have produced;GenerationTally(add_v3), else/v1/statsreports BitNet traffic as zero throughput;SSE_DONE— the response stream already chains it, so the original would have emitted it twice.Verification
Pinned 1.98.0, on a 32-vCPU machine rather than my laptop (which produces spurious SIGSEGVs in this crate's integration binaries — on both this branch and unmodified
main, so not from these changes):cargo clippy -p larql-server --all-targets -- -D warnings: clean.--all-targetsmatters here — sevenLoadedModelliterals intests/need the two new fields, andcargo buildalone does not see them.cargo test -p larql-server --no-fail-fast: 1279 passed, 0 failed, 0 crashes, against 1277 passed, 0 failed, 0 crashes on unmodified23a56db1. The +2 are the two tests added here (is_dense_only_detects_empty_gate_layers,bitnet_model_not_loaded_by_default).I have not tested this against the real
microsoft/bitnet-b1.58-2B-4Tcheckpoint on this rebase — the original work was verified end-to-end against it (Paris 94.5%, and a no-moderequest returning the same as explicitmode:dense), but that was on the June tree. Worth a run against real weights before merging if you have one handy.