feat: add the rustqc dna subcommand (depth of coverage, samtools, preseq) - #153
Open
BenjaminDEMAILLE wants to merge 15 commits into
Open
feat: add the rustqc dna subcommand (depth of coverage, samtools, preseq)#153BenjaminDEMAILLE wants to merge 15 commits into
rustqc dna subcommand (depth of coverage, samtools, preseq)#153BenjaminDEMAILLE wants to merge 15 commits into
Conversation
These three modules carry no RNA-specific logic and are needed by the forthcoming dna subcommand. src/rna re-exports them so every existing crate::rna::... path and the published 0.2.x library surface keep working. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bam_stat is read-level and needs no annotation, and the samtools stats, flagstat and idxstats writers consume its result type, so all four move together into src/common/. src/rna/rseqc re-exports them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BamStatAccum gathers the read-level counters behind bam_stat and the samtools writers. Its process_read takes only a record and a MAPQ cutoff, so it is assay-agnostic and the dna pipeline will drive the same struct. The merge_vec_arrays helper moves with it, being its only consumer. rna::rseqc::accumulators re-exports the type. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Also corrects the AGENTS.md claim that the crate has no lib.rs, which has been untrue since seqeralabs#101. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A real public human chr22 slice from nf-core/test-datasets, duplicate-marked locally with samtools, plus mosdepth 0.3.14 and samtools 1.24 reference outputs. The generation script pins both tool versions and refuses to run against others, so fixtures and tool versions cannot drift apart. 380 kB in total, well inside the 10 MB fixture budget. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Shared options keep the same long name, short flag and RUSTQC_* environment variable as their rna counterparts. The deliberate differences: no --gtf, no --stranded, and --mapq defaults to 0 rather than 30 because that is mosdepth's default. run_dna is a stub for now; the pipeline lands in the following commits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Config gains a `dna` block alongside `rna`, with mosdepth and samtools sub-sections and a reuse of the existing PreseqConfig. Shared settings (chromosome_prefix, chromosome_mapping, sample_name, flat_output) are declared on DnaConfig itself, mirroring RnaConfig, so the two pipelines can be configured independently in one file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DepthAccum records aligned blocks as increments in a delta array the length of the contig, then a prefix sum turns that into per-base depth in one linear pass. Filters and CIGAR handling reproduce mosdepth 0.3.14 outside fast mode: flags 1796 excluded, MAPQ floor applied, M/=/X cover the reference, D/N advance without covering, and I/S/H/P do not advance. Mate-overlap correction, the other half of mosdepth's default behaviour, lands in the next commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mosdepth counts a base once when both mates of a pair cover it, unless --fast-mode is given. On the test dataset this is the difference between 469875 and 247878 total covered bases, so it is the dominant behaviour rather than an edge case. Pending mates are held in a map keyed by read name, indexed by the position the outstanding mate was announced at so entries that can never be claimed are evicted as the coordinate-ordered scan moves past them. A test asserts the map empties. Includes an engine-level parity check against the committed mosdepth fixture: total covered bases and maximum depth both match exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six writers plus the per-contig summarisation that feeds them: summary, global and region distributions, per-base runs, per-window means and per-window threshold counts. Compressed outputs are bgzf, matching mosdepth. The distribution emission rule was reverse-engineered from the fixtures and is the non-obvious part: every depth from 0 up to min(300, max) gets a row whether or not any base sits at it, above 300 only depths that occur and lie strictly below the maximum do. So the maximum gets a row when it falls inside the dense range and none when it does not. The global distribution tops out at 866 with a maximum of 867, while the region distribution does emit its maximum of 204. The region distribution is over windows and their rounded mean depth, not over bases. Parity tests drive the library directly and compare every mosdepth output against the committed fixtures: all eight match, including the 1094-line global distribution and the 721-interval per-base BED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One rayon worker per contig, each holding its own depth array, feeding a DepthAccum, a BamStatAccum and a PreseqAccum from the same record stream, so the alignment is read once. A separate pass over unmapped records feeds the counters flagstat and idxstats report. Workers run longest contig first and their number is bounded by --max-depth-workers, defaulting to a 4 GB budget divided by the largest contig, because each worker costs four bytes per base. Outputs land under mosdepth/, samtools/ and preseq/, or flat with --flat-output. Input without duplicate marks is rejected unless --skip-dup-check is passed. Also fixes the samtools stats header, which hardcoded "rustqc rna" and so labelled DNA output as RNA output. End-to-end parity tests run the binary and compare against the fixtures: all six mosdepth files match byte for byte, flagstat and idxstats match exactly, and all 1889 data lines of samtools stats match. The stats header differs by design, RustQC naming itself rather than reproducing samtools' version banner, so that comparison is on data lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
InputSummary gains an optional dna block carrying genome length, covered bases, mean, median and maximum coverage, the percentage of the reference at or above each requested threshold, and the duplicate rate. An input carries either the RNA fields or this one, never both. Coverage thresholds are a list of objects rather than a map so the requested order survives serialisation; a map keyed by the threshold would sort "10" before "5". CITATIONS.md for a dna run cites mosdepth, samtools and preseq, and none of the RNA-only tools. The header is now shared between both writers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n used Closes two gaps left open in this PR. The bgzf BED outputs now get a .csi index built through htslib's tbx_index_build, as mosdepth writes and as tabix needs to seek into them. CSI rather than TBI because CSI carries no 512 Mb coordinate ceiling. Indexes are not compared byte for byte: an index is binary metadata over the compressed blocks, and two writers answering the same queries need not produce the same bytes. The test asserts instead that a region query returns the same rows through our index as through mosdepth's, going through the tabix binary because rust-htslib's tabix reader ends a fetched region with a TabixTruncatedRecord rather than stopping, and does so at different points for the two files. It skips where tabix is absent. CITATIONS.md for a dna run now cites samtools v1.24, the version its fixtures were generated with, instead of the v1.22.1 the rna pipeline was validated against. Each pipeline cites the version it was actually compared with rather than both claiming the newer one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 28, 2026
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.
Part of #157.
Second of the DNA stack. Stacked on #152, which extracts the shared modules this builds on; review that one first, and the diff here will shrink once it merges.
Adds
rustqc dna <BAM>...: a single-pass DNA QC pipeline that needs no annotation.What it does
One rayon worker per contig, each holding its own depth array and feeding a depth accumulator, a
BamStatAccumand aPreseqAccumfrom the same record stream, so the alignment is read once. A separate pass over unmapped records feeds the counters flagstat and idxstats report.Outputs land under
mosdepth/,samtools/andpreseq/, or flat with--flat-output, alongsiderustqc_summary.jsonandCITATIONS.md.Parity
Every output is compared against the real upstream tools on a committed fixture, a real public human chr22 slice from nf-core/test-datasets, duplicate-marked locally:
mosdepth.summary.txtmosdepth.global.dist.txt(1094 lines)mosdepth.region.dist.txtper-base.bed.gz(721 intervals)regions.bed.gz,thresholds.bed.gzflagstat,idxstatssamtools statssamtools statsheaders differ by design: RustQC writes its own#banner naming itself rather than reproducing samtools' version string. That is pre-existing behaviour shared with thernapipeline. This PR does fix a bug in it, though: the banner hardcodedrustqc rna, so DNA output was labelled as RNA output.The fixture generation script pins mosdepth 0.3.14 and samtools 1.24 and refuses to run against other versions, recording them in a
VERSIONS.txtthe test suite asserts against, so fixtures and tool versions cannot drift apart.Two things worth knowing, both measured rather than assumed
Mate-overlap correction is the dominant behaviour, not an edge case. mosdepth counts a base once when both mates of a pair cover it, unless
--fast-modeis given. On this dataset that is the difference between 469875 and 247878 total covered bases, a factor of two. It has its own commit so it can be reviewed on its own.The distribution files have a non-obvious emission rule, reverse-engineered from the fixtures and checked against every row: every depth from 0 up to
min(300, max)gets a row whether or not any base sits at it, and above 300 only depths that occur and lie strictly below the maximum do. So the maximum gets a row when it falls inside the dense range and none when it does not. The global distribution tops out at 866 with a maximum of 867, while the region distribution does emit its maximum of 204. The region distribution is also over windows and their rounded mean depth, not over bases.Defaults
-Q/--mapqdefaults to 0 fordna, matching mosdepth, rather than the 30rnauses. Every other shared flag keeps itsrnaname, short form andRUSTQC_*environment variable.--max-depth-workersbounds how many contig depth arrays are live at once. Each worker costs four bytes per base, about 1 GB for GRCh38 chr1, so the default is a 4 GB budget divided by the largest contig. There is no portable way to read free memory from std, so that budget is a constant; the flag is the escape hatch.Not in this PR
Picard metrics (
CollectWgsMetrics,CollectInsertSizeMetrics,CollectGcBiasMetrics,CollectHsMetrics) and Qualimapbamqcland in the following PRs.--targetsis accepted and warns that targeted metrics are not implemented yet. The.csicompanion indexes for the bgzf outputs are not written yet either.One inconsistency to flag:
CITATIONS.mdcites samtools v1.22.1, the version constant the RNA pipeline was validated against, while these fixtures were generated with 1.24. I left the constant alone rather than silently claiming the RNA pipeline was revalidated.Tests
283 green, up from 233. Includes 15 parity tests, and unit tests covering the paths the fixture cannot reach, notably the MAPQ filter:
mosdepth -Q 30returns the same totals as the default on this data, so that path is covered on synthetic records instead.🤖 Generated with Claude Code