feat(demux): fused CTC-CRF pipeline, self-describing bundles, LLR opt-in - #172
Merged
Conversation
Three changes, all aimed at `escpod demux in.pod5 --model <bundle> -d out/` producing barcoded POD5s with nothing else on the command line. The fused pipeline drove only the fingerprint heads (DTW-SVM, GBM); the CRF was reachable only as `demux basecall`, which needs boundaries fed in, so using it meant detect -> basecall -> split with two intermediate files and three passes over the POD5. `--model` now accepts a CRF bundle directory and runs the CRF inline: detect -> prep the raw-pA window -> basecall -> match by edit distance -> route, decoding each read once. Verified against the 3-step path on 4,000 reads, same detector/encoder/refs: all 3,993 shared reads get an identical call, and every barcode bin matches exactly. The fused path additionally emits 4,000 rows to the 3-step path's 3,993 — `basecall` drops reads with no usable window, so `split` never sees them, whereas the fused path routes them `unclassified` like the other heads. Output now reconciles with input. `metadata.json` gains optional `barcodes`, `boundary`, `model` and `metrics`. References in the bundle are what the model EMITS (`target[state_len:]`), which removes a real trap: a hand-written CSV of full-length targets still calls the same barcode but inflates every distance by `state_len` and compresses the confidence margin that `--min-margin` and `--recovery` rank on (escapepod-models#36). `--barcodes` remains, as an override. New `--info` prints identity, geometry, references, pinned detector, published metrics and caveats, then exits without touching a POD5 — so a model can be interrogated before it is trusted. `--method` had `default_value = "llr"`. LLR boundaries cost 17.2 points of barcode recall against the same classifier (0.9928 -> 0.8196) and the failure is silent (escapepod-models#16). It now has no default: - a bundle may pin its detector, and supplies both method and weights; - an explicit `--method` overrides that pin, except that a bundle pinned to `cnn` refuses `--method llr` (the runtime guard #16 asks for); - with neither, both `demux` and `demux detect` error out naming the tradeoff rather than quietly picking the worse detector. BREAKING: `escpod demux` and `escpod demux detect` now require `--method` when the model does not pin one. Scripts relying on the implicit `llr` default must add `--method llr` to keep their current behaviour — which is the point, since that default was silently costing 17.2 points. 156 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jayhesselberth
force-pushed
the
feat/fused-crf-and-model-info
branch
from
July 31, 2026 23:57
026c825 to
d2143ad
Compare
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jayhesselberth
added a commit
that referenced
this pull request
Aug 1, 2026
Two independent findings from a performance audit of the CTC-CRF path added in #163/#164/#165/#172. ## 1. `produce_cpu_crf` could not fill the machine The CRF producer fanned out with `.chunks(256)` and then walked each chunk with a plain serial `for`. That shape was copied from `produce_cpu_gbm`, but that head chunks for a reason that does not apply here: `predict_many` is a genuinely batched kernel, whereas tract has no batched LSTM, so the CRF chunk only ever serialized its reads. The chunk existed solely to amortize `CrfScratch::new()`. Two consequences, both measured: - At ~14 ms per read (13 ms encode + 1.2 ms decode on `rna`), one chunk is **~3.6 s of work that a starved worker cannot steal**, so every block tail idles cores for that long. - A block with fewer than `256 × threads` reads **cannot fill the machine at all** — 1000 reads produced exactly four tasks for 32 cores. Replaced with `for_each_init(CrfScratch::new, …)`, which gives per-read work stealing *and* moves `CrfScratch` from per-chunk to per-worker. This is the shape `produce_cpu` and `demux basecall` already use. | Input | Before | After | Speedup | |---|---|---|---| | 1k reads | 4.91 s | 2.19 s | 2.25× | | 10k reads | 23.3 s | 17.0 s | 1.36× | (32 cores on `rna`. The gap narrows as the chunk count catches up to the core count, which is exactly the predicted shape.) ## 2. `basecall_batch` retained every read's scores `CrfEncoderGpu::basecall_batch` encoded the entire caller batch before decoding any of it. `ESCAPEPOD_CRF_GPU_BATCH_ROWS` bounds the *device*-side activations, not the host — the scores coming back are `t_len * n_score` floats, **1 MB per read** for the RNA004 geometry, so an Arrow batch of a few thousand reads retained gigabytes of host RAM no matter what that knob was set to. Encode and decode now alternate one device batch at a time, capping host high-water at `batch_rows` reads' worth. Splitting is already exact and tested (`run_batch` documents why), and the chunk boundaries are unchanged, so the scores are identical. `run_batch` now takes `&[&[f32]]` rather than `&[Vec<f32>]`, which also drops the per-read 8 KB window clone on the way to the device. Note this does *not* overlap encode with decode — that would need a proper pipeline. It is a memory fix. ## Verification Full workspace suite (460 tests) and doctests pass; `cargo fmt --check` and `clippy -D warnings` clean on both default features and `crf-gpu`. End-to-end against the nbc16 RNA004 CRF bundle at 1k and 10k reads: - `--classifications` CSVs **identical** after sorting (row order is rayon-nondeterministic by design) - all 14 per-barcode POD5 outputs **byte-for-byte identical** - per-barcode summary counts unchanged ## Not addressed here The audit turned up several other items left for follow-ups: the `split_time_major` copy-out still runs while holding the session mutex; `adc_to_pa` converts the whole decoded prefix when `prep` uses only the last 2000 samples; `BLOCK_TARGET_BYTES` is a per-filler bound rather than the global cap its comment implies; the router slot budget is only honored up to 192 barcodes; and `barcode.rs`'s WFA cap is set to the maximum possible score, so the early abandonment its docstring describes never actually fires.
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.
Makes
escpod demux in.pod5 --model <bundle> -d out/produce barcoded POD5s with nothing else on the command line, and adds--infoso a model can be interrogated before it is trusted.Fused CRF head
The fused pipeline drove only the fingerprint heads (DTW-SVM, GBM). The CRF was reachable only as
demux basecall, which takes boundaries as input, so using it meantdetect→basecall→split: two intermediate files and three passes over the POD5.--modelnow accepts a CRF bundle directory (sniffed by the sidecar'sformatkey, not the extension) and runs the CRF inline — detect → prep the raw-pA window → basecall → match by edit distance → route — decoding each read once.Verified against the 3-step path, 4,000 reads, same detector, encoder and references:
basecalldrops reads with no usable window, sosplitnever sees them; the fused path routes themunclassifiedlike the other heads. Output now reconciles with input.Bundles describe themselves
metadata.jsongains optionalbarcodes,boundary,modelandmetrics. So this:becomes:
Carrying references in the bundle is not only ergonomics. The CRF has
state_len=4and emitstarget[4:], so a hand-written CSV of full-length targets still calls the same barcode but inflates every edit distance by 4 and compresses the confidence margin that--min-marginand--recoveryrank on (rnabioco/escapepod-models#36). Measured on 20,000 reads with the shipped weights: medianbest_dist4 → 0, margin median 10 → 12/13, distinct margin values 12/11 → 15/14. Deriving them at bundle-build time from the encoder's ownstate_lenremoves the failure mode rather than documenting it.--barcodesremains as an override.New
--infoprints identity, geometry, references with their minimum pairwise distance, the pinned detector, published metrics and caveats, then exits without touching a POD5.LLR is opt-in, never inferred
--methodhaddefault_value = "llr". LLR boundaries cost 17.2 points of barcode recall against the same classifier (0.9928 → 0.8196) and the failure is silent — it runs and produces plausible output (rnabioco/escapepod-models#16). It now has no default:--methodoverrides that pin — except a bundle pinned tocnnrefuses--method llr, which is the runtime guard perf: update bam-filter to use optimized filter_files() #16 asks for;demuxanddemux detecterror out naming the tradeoff instead of quietly picking the worse detector.Warning
Breaking.
escpod demuxandescpod demux detectnow require--methodwhen the model does not pin one. Scripts relying on the implicitllrdefault must add--method llrto keep current behaviour — which is the point, since that default was silently costing 17.2 points.Interaction with #166
#166 touches
demux/detect.rsanddemux/mod.rs, as does this. The conflicts are textual and small (it adds--cnn-model-name; this changes--methodto anOption), but they do need resolving. #166 is older and independent, so merging it first and rebasing this is the natural order.Worth reconciling separately: both PRs use the word bundle for different things — #166's is a release archive of matched models with a
BUNDLE.json, this one's is a directory the CRF loader reads viametadata.json. They are complementary, but the naming will confuse someone.Testing
156 tests pass. Adds a unit test that
--modelsniffing keys off the sidecar'sformatfield rather than the path, so a classifier JSON beside a bundle, a directory without a sidecar, and a malformed sidecar all still route toload_any_model.Manually verified: the fused CRF short form, the long form with explicit flags, GBM regression through the fused path, and all four guard paths (llr-vs-pinned-cnn, no-method-no-pin,
detectwith no method, and explicit llr opt-in on an unpinned model).🤖 Generated with Claude Code