feat: add align subcommand for single-pass DNA alignment QC - #136
Open
BenjaminDEMAILLE wants to merge 1 commit into
Open
feat: add align subcommand for single-pass DNA alignment QC#136BenjaminDEMAILLE wants to merge 1 commit into
BenjaminDEMAILLE wants to merge 1 commit into
Conversation
Sarek reads each sample's CRAM 6-7 times during QC. This collapses three of
those passes into one: samtools stats, mosdepth-equivalent depth, and the
bcftools mpileup genotyping step that feeds NGSCheckMate all come out of a
single streaming pass.
The genotyping is nearly free. The NGSCheckMate panel is ~10K positions
across ~3 Gb, so for a coordinate-sorted file the check is a single
comparison for almost every read; only the reads that actually overlap a site
need CIGAR-resolved base extraction. bcftools mpileup instead builds complete
pileup columns genome-wide and runs a genotype likelihood model.
Depth is exact, from CIGAR-aware start/end delta events (M/=/X/D contribute,
N skips leave a gap). Because input is sorted, depth is finalised as the file
streams past, so memory scales with pile-up depth rather than genome size.
Records excluded: unmapped, secondary, QC-fail, duplicate (mosdepth's default
--flag 1796).
Outputs: {sample}.stats / .flagstat / .idxstats (reusing the accumulator the
rna command already validates against samtools), .mosdepth.summary.txt,
.mosdepth.global.dist.txt, .regions.bed.gz and .ngscheckmate.vcf.gz.
All three depth outputs are byte-identical to mosdepth 0.3.x on the test
dataset, including its quirks: the <chrom>_region summary rows emitted with
--by, and the 8e-5 cumulative cutoff that skips the sparse tail of the
distribution.
Genotypes come from the alternate allele fraction (<0.15 hom-ref,
0.15-0.85 het, >0.85 hom-alt) since ncm.py only needs those three states.
The SNP BED must carry ref/alt alleles; a shorter BED is rejected rather than
guessed at, because per-sample VCFs have to share a common allele set.
Scope: phases 1 and 2 of the proposal. --by takes a fixed window size only
(no target BED), there is no per-base.bed.gz or .csi index, and genotyping
does no BAQ recalculation. The docs say so explicitly.
Refs seqeralabs#18
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Addresses #18 (phases 1 and 2 of the proposal). Not marked
Closes— phase 3 and the pipeline-side work remain.rustqc aligncollapses three independent passes over the same CRAM/BAM into one:{sample}.stats/.flagstat/.idxstatssamtools statsetc.{sample}.mosdepth.summary.txtmosdepthsummary{sample}.mosdepth.global.dist.txtmosdepthdistribution{sample}.regions.bed.gzmosdepthper-window depth{sample}.ngscheckmate.vcf.gzbcftools mpileup | bcftools callncm.pyThe samtools outputs reuse the accumulator the
rnacommand already validates against samtools, so this PR is mostly the depth engine and the genotyping.mosdepth output is byte-identical
Verified against
mosdepth 0.3.x(mosdepth --by 500 md tests/data/test.bam) on the test dataset —summary.txt,global.dist.txtand the decompressedregions.bed.gzalldiffclean. That required matching two mosdepth behaviours that are easy to miss:<chrom>_region/total_regionsummary rows emitted whenever--byis usedif cum < 8e-5: continuecutoff inwrite_distribution, which skips the sparse tail (chr2's max depth is 40, but 39 is the deepest row mosdepth prints)Both are pinned in an integration test and flagged in AGENTS.md so they don't get "cleaned up" later.
Depth engine
Exact, from CIGAR-aware start/end delta events:
M/=/X/Dcontribute,Nskips leave a gap. Since input is coordinate-sorted, every base before the current read's start is final and gets folded in as the file streams past — memory scales with pile-up depth, not genome size. Excluded records match mosdepth's default--flag 1796(unmapped, secondary, QC-fail, duplicate).Genotyping
For a sorted file the SNP panel is a sorted array, so the per-read cost is one comparison; only reads that overlap a site get CIGAR-resolved base extraction (handling soft clips, insertions, deletions and
Nskips — covered by unit tests).Genotypes come from the alternate allele fraction (
<0.15→0/0,0.15–0.85→0/1,>0.85→1/1), which is allncm.pydistinguishes.FORMATisGT:AD:DP.The SNP BED must be the 6-column NGSCheckMate layout including ref/alt alleles. A shorter BED is rejected with an explicit error rather than having alleles inferred — inferring them per sample would let two samples disagree on REF/ALT and silently corrupt the correlation matrix.
Not covered
--bytakes a fixed window size only; a target BED (mosdepth's--by <bed>) is not supported, so WES target-restricted coverage still needs mosdepthper-base.bed.gz, no.csiindex forregions.bed.gzbcftools mpileupin repetitive regionsThe validation the issue asks for — comparing NGSCheckMate VCFs and the resulting
ncm.pycorrelation matrices on real data — has not been done here; the test dataset has no real variation. That is the piece to run before anyone swaps this into a pipeline, and I'd suggest treating the genotyping as unvalidated until then.Note this shares machinery with #128 (bamqc): both need an exact streaming depth tracker. If both land, the two are worth unifying.
cargo test— 210 lib + 20 integration tests passcargo fmt --check,cargo clippy -- -D warningscleandocs/align, AGENTS.md updated🤖 Generated with Claude Code