Repository navigation
fix(bam): count unplaced reads once and handle empty CRAM sweeps - #177
Draft
NicoDeVeaux wants to merge 6 commits into
Draft
NicoDeVeaux wants to merge 6 commits into
NicoDeVeaux wants to merge 6 commits into
Conversation
On the indexed (parallel) path, `fetch(Unmapped)` seeks to the last linear-index window of the last reference and keeps every UNMAP-flagged record, so a placed unmapped mate in that window is read by both its reference's fetch and the sweep. htslib's HTS_IDX_NOCOOR returns only records with no reference (tid -1). - Unit tests: BAM (noodles-written) and two samtools-written CRAMs (per-reference slices, one multi-reference slice); the sweep must return exactly the two unplaced records. - Integration test: flagstat at --threads 1 and 4 must match samtools 1.22.1; at 4 the placed mate is counted twice (in total 7, not 6). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`fetch(Unmapped)` now keeps a record only when it has no reference sequence ID, matching htslib's HTS_IDX_NOCOOR: - BAM: the sweep seeks to the last reference's final linear-index window, so it re-read placed records there and kept every UNMAP-flagged one; a placed unmapped mate in that window was counted twice on the parallel path (flagstat "in total" 7 instead of 6 in the new test). - CRAM: `query_unmapped` starts at the slice holding the unplaced reads, which in a multi-reference slice also holds placed records; the same filter applies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Port the verified iterator-only delta from a9083c5. Preserve integer addition and channel order. Clears the three existing needless_range_loop warnings on the pinned native toolchain.
NicoDeVeaux
changed the title
Use the equivalent Option propagation required by current Clippy. Preserve cursor exhaustion and seek-error behavior.
Pin crossbeam-epoch 0.9.20 using the exact version and checksum already validated in the combined candidate. Leave all other lock entries unchanged.
This branch has not been deployed
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.
The parallel reader now returns each unplaced record once: the BAM and CRAM unmapped sweeps keep records with no reference ID, excluding placed unmapped mates already returned by a reference fetch. CRAM files with no unplaced CRAI entry now return an empty sweep instead of seeking EOF and failing with
failed to fill whole buffer.This targets the #118 noodles migration (
0b00cf), and remains based onnoodles-migration-redo. #178 is stacked on this branch and supplies the separate multi-reference and same-reference multi-slice CRAM fixes. Both fixes should be incorporated into #118 before that migration lands; they remain separate review units here.The original six-record placed-unmapped example changes from seven records at four threads on #118 to six, matching samtools and the one-thread run. A new valid, indexed placed-only CRAM reproduces the empty-sweep failure on the previous version of this PR and passes after the guard.
Validation of
f45947on macOS arm64 / Rust 1.90.0:A separate CI follow-up pins crossbeam-epoch 0.9.20 and replaces an equivalent iterator-exhaustion match with
?for newer stable Clippy. Fresh Linux/upstream CI and security-audit results remain separate checks. The Linux scientific and downstream evidence for #175'sa9083capplies to that counting candidate; this migration stack has its own validation. Combining the branches requires conflict resolution and another verification run. This PR does not decide empty-input or no-genic-read exit policy.This update is written by GPT-6 on Nico DeVeaux's behalf. The original PR was prepared with Claude Code.