fix(prepare): don't count reads absent from a pre-filtered POD5 as failures - #328
Merged
Merged
Conversation
…ilures A BAM covering the full reference set paired with a POD5 deliberately filtered to a subset of those reads (escpod bam-filter, the pipeline's own Filter stage) had every absent read counted as a per-read failure on both backends. Past 50% of the BAM -- which any selective filter reaches -- the run raised on MAX_FAILED_READ_FRACTION and produced nothing, although every read that was present extracted correctly. Absent reads are now their own expected-exclusion bucket, alongside "no motif match": BatchOutcome.n_reads_missing_from_pod5, stats["reads_missing_from_pod5"], and a figure in the "Read yield" line. The failed fraction is measured over the reads actually attempted, so a genuinely broken run cannot hide behind a filtered POD5 either. Issue #265's zero-tolerance policy is preserved. Telling the two apart needs Rust's help, since training.rs drops an absent read to zero chunks exactly as a broken pipeline would: leech_core.extract_training_chunks now returns (chunks, n_missing_from_pod5), and _iter_rust_batches' zero-yield heuristic fires on n_submitted - n_missing, so reads that were found and still produced nothing remain failures. Closes #325.
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
escpod bam-filter, this project's own pipeline Filter stage — had every absent read counted as a per-read failure on both backends. Once the (by-design) mismatch passedMAX_FAILED_READ_FRACTION(50%), which any selective filter reaches,leech data prepareraised and produced no output at all, although every read that was in the POD5 extracted correctly.reads_without_motifrather than insidefailed_reads:BatchOutcome.n_reads_missing_from_pod5,stats["reads_missing_from_pod5"], and a figure in the "Read yield" log line. The Python fallback's per-readwarningfor this became adebug— on a pre-filtered POD5 it fires for most of the BAM, and the run-level count is the number worth reading.total_reads - missing) in both numerator and denominator. Leaving absent reads in the denominator is the mirror bug: 100 reads present with 60 of them failing shows as 6% of a 1000-read BAM and never trips.training.rsdrops a read whose id is not in the signal map to zero chunks, so from Python a pre-filtered batch and a batch the pipeline silently broke on are indistinguishable — which is exactly what_iter_rust_batches' "submitted reads, no chunks back" heuristic (rust: return struct-of-arrays per batch and per-read outcome counts instead of a list of dicts #267/rust: replace the local chunk pipeline withescapepod_signal::chunk#258) keys off.leech_core.extract_training_chunkstherefore returns(chunks, n_missing_from_pod5)instead of a bare chunk list (_process_and_convert_trainingcounts submitted ids absent from the signal map), and the heuristic now fires onn_submitted - n_missing. Aleech_corepredating the change is caught with a "rebuild the extension" error rather than a cryptic unpack failure.handle_prepareraises on, and that message now names the absent-read count so the mistake doesn't read as a motif problem.CLAUDE.md's "Failing loud on a broken prepare run (issue prepare: a run that loses every batch exits 0; a backend divergence hides in one config corner; torch is imported before the pool forks #265)" section documents the new category, the attempted-reads denominator, and the Rust return-shape change;changelog.d/325.fixed.mdadded.Closes #325.
Test plan
Rust extension rebuilt before any Python test that exercises it, and every command below run under
srun -p rnarather than on the login node.srun ... --chdir .../rust -- uv run maturin develop --release(i.e.rust/build.sh) — clean build of the new(chunks, n_missing_from_pod5)signaturecargo fmt --check— cleancargo clippy --all-targets -- -D warnings— cleancargo test --no-default-features --features test-utils— 16 passeduv run pytest tests/test_parallel_prep.py tests/test_prepare_dispatch.py tests/test_backend_parity.py -v— 128 passeduv run pytest(full suite) — 1931 passed, 45 skippeduv run ruff format --check ./uv run ruff check .— cleanuv run ty check src/leech/— cleanNew regression tests, on both backends:
TestPreFilteredPod5(tests/test_parallel_prep.py) writes a real POD5 holding 2 of the fixture's 20 reads (viaescapepod.Writer), then runs the full BAM against it:n_failed == 0,n_missing == 18(90% of the batch), chunks bit-identical to the unfiltered run for the two present readsn_failed == 0,n_missing == n_submitted - 2, chunks only for the present readsprepare_training_data_parallelend to end on Rust (--backend rust,chunk_size=2, so most batches hold no present read at all) and on Python (realmp.Pool, in a fresh subprocess per the repo's fork/rayon rule): both succeed,failed_reads == 0, absent reads > 50% of the BAM. Reference-anchored, deliberately — underbasecallonly 9 of 20 reads submit, so the Rust path could never reach the threshold from this fixturefailed_reads(2) and not in the absent bucket (18); at the dispatcher,([], 0, 3, 1)(one absent, two found and yielding nothing) still counts 2 failed, while([], 0, 3, 3)counts none; at the run level, 8 failures behind 10 absent reads of 20 still raises, and the attempted-reads denominator is pinned by a case that passes on the old denominator and fails on the new one🤖 Generated with Claude Code