Repository navigation
fix(flagstat): match samtools counts, alignment priority and percentages - #175
Draft
NicoDeVeaux wants to merge 5 commits into
Draft
NicoDeVeaux wants to merge 5 commits into
NicoDeVeaux wants to merge 5 commits into
Conversation
…plicate counts Failing tests for three divergences from samtools flagstat 1.22.1: - QC-failed reads (0x200) are counted in the QC-passed column; the second column is always 0. - "properly paired" counts a PAIRED|PROPER_PAIR record that is itself unmapped (samtools requires !UNMAP, bam_stat.c:61). - A QC-failed duplicate (0x600) is in neither "duplicates" column, while "primary duplicates" counts it. The unit tests check every 12-bit flag against a transcription of samtools' flagstat_loop; the integration tests compare `rustqc rna` output with golden output from samtools 1.22.1. The SECONDARY|SUPPLEMENTARY line (#125) is kept at its current behaviour and pinned as such. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…stat_loop flagstat now keeps its own `[QC-passed, QC-failed]` counters (`FlagstatCounts`), counted like samtools' `flagstat_loop` (bam_stat.c): - QC-failed reads (0x200) go in the second column, with their own percentages, instead of being folded into the first. - "properly paired" requires the read itself to be mapped. - "duplicates" counts every 0x400 record, not the RSeQC bam_stat cascade counter that drops QC-failed duplicates. The SECONDARY|SUPPLEMENTARY case (#125) is unchanged: such a record still counts on both lines. The counters that only fed flagstat are removed from BamStatAccum/BamStatResult; those samtools stats reads are untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Port PR #131 secondary priority to both legacy and QC-split counters. Match the pinned samtools C percentage arithmetic, restore unmodified exhaustive oracles, and cover QC-failed duplicate overlaps and partition merges. Select the Cargo-provided integration binary at compile time so release tests keep their profile and CARGO_TARGET_DIR. Add a raw production-counter example for independent differential validation. Fixes #125
Replace the three existing needless_range_loop findings with fixed-size iterator zips. Preserve cycle/channel order and u64 addition order; floating percentage calculations are unchanged. Validated with cargo fmt --check, strict cargo clippy, and the complete 244-test release suite.
NicoDeVeaux
changed the title
This was referenced Nov 1, 2026
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.
QC-failed records now populate the second flagstat column, unmapped records cannot count as properly paired, and QC-failed duplicates remain duplicates. The flagstat writer and the legacy stats accumulator now both give SECONDARY priority over SUPPLEMENTARY, so dual-flagged records are counted once.
samtools statsreports secondary records alone asnon-primary alignments, and percentage division follows samtools' C float expression before formatting.This extends the secondary-priority and stats semantics proposed by Benjamin Demaille in #131 to the separate
FlagstatCountspath introduced here. Merging the previous versions of #131 and this PR unchanged would leave the new counter double-counting. #131 remains open for maintainer coordination; adopting the samtools code's precedence for #125 still needs maintainer agreement.The regression tests no longer modify the oracle to preserve the old overlap behavior. They retain the original samtools-generated all-flags fixture, cover overlap combined with QC-fail/duplicate flags, compare partitioned counter merges, and check large-count rounding. Integration tests use Cargo's actual test binary rather than falling back to an unrelated debug build. The branch also includes the recorded crossbeam-epoch/libz-sys lockfile updates and a separate iterator-only fix for three existing Clippy warnings.
Validation of candidate
a9083c:This is targeted flagstat/stats evidence. It does not establish complete RNA-QC equivalence, Wave/container packaging, a full rnaseq run, genuine single-end coverage or strand-selection equivalence. Other stats fields, including
reads properly paired, have separate definitions and are outside these targeted repairs. The wider frozen campaign retains five old-baseline expectation failures, and empty input still exits zero with one thread but nonzero with 2/4/8; no expectations were changed after scoring. The separate living review has two scientific PASS gates and nine pending recipes, so full merge/release acceptance remains UNKNOWN. Its byte/source/build records are trusted evidence, without machine or source-to-binary attestation.This counting PR stays independent of the #118 noodles migration and its stacked #177/#178 fixes.
This update is written by GPT-6 on Nico DeVeaux's behalf. The original PR was prepared with Claude Code.