Repository navigation
bamRemoveDuplicates: a mateless unique read keeps the cluster open, as STAR - #370
Open
BenjaminDEMAILLE wants to merge 1 commit into
Open
BenjaminDEMAILLE wants to merge 1 commit into
BenjaminDEMAILLE wants to merge 1 commit into
Conversation
STAR compares POS and PNEXT as uint32 when it builds the clusters, so a unique read without a mate (PNEXT -1) sets the cluster's right end to 0xFFFFFFFF and holds it open to the end of the reference. rustar compared them as signed, ended the cluster earlier, and could leave such a read unpaired and marked where STAR keeps it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Part of #354 (
--bamRemoveDuplicatesType: which pair STAR keeps).PR base:
perf/bam-dedup-streaming(#363). That PR rewrotesrc/bam_dedup.rsto stream, so this fix is on the new code. Please merge #363 first; only the last commit is this PR.Problem
The task was: rustar keeps another pair than STAR among duplicates with the same
AS. I tested that against STAR 2.7.11b built on Linux, and ties already match. The tie difference only shows up against the macOS build of STAR (see below). What did differ is something else: a unique read without a mate (single-end,PNEXT0 in SAM, -1 in BAM) next to paired reads. STAR keeps it; rustar marked it as a duplicate.What STAR does (2.7.11b source)
All in
bamRemoveDuplicates.cpp.qsort(line 206;funCompareNames, line 13: name length, then bytes, then the0x80bit), then pairs are sorted withqsort(line 207) byfunCompareCoordFlagCigarSeq(line 72). In each run of equal pairs, the first pair with a strictly higherASwins (lines 209, 223-226). So on a tie, the winner is the first pair in the sorted order.qsortis a stable merge sort (the Linux image runs glibc 2.36). So equal pairs stay in name order, and the tie goes to the smallest name. rustar'ssort_byis stable too, so it gives the same answer.qsortis not stable./opt/homebrew/bin/STARkeeps another pair on ties (for examplehiinstead ofb9, bothAS30, same key). I took the Linux build as the reference.leftE,rightEandrightMaxareuint32(lines 141, 173-174, 199-200). A cluster ends whenleftE > rightMax(line 205). A unique read withrightE > leftEraisesrightMax(line 258). WithPNEXT-1,rightEis 0xFFFFFFFF, sorightMaxbecomes 0xFFFFFFFF and the cluster runs to the end of the reference.grN/2pairs), so a mateless read pairs with whatever comes next by name.Change
src/bam_dedup.rs: comparePOS/PNEXTasu32when building clusters, as STAR does. Three lines.Measured against STAR 2.7.11b
STAR built for Linux (Docker, glibc 2.36). Each BAM was run with
UniqueIdenticalandUniqueIdenticalNotMulti. I comparedsamtools viewmd5. Forsorted1I also compared the md5 of the decompressed BAM (gzcat): identical. Records that differ = records whose name/flag/chr/pos differ from STAR.ASties, names out of order and of different lengths, a multimapper,2S18Msame key, other flags, 2 refscase2, before:
STAR has one cluster for all of chr1, so
se1andse2pair up by name and are kept. rustar split the cluster at the gap, so each of them was left alone and stayed marked.Tests
a_mateless_unique_read_keeps_the_cluster_open: the case2 shape. It fails without the fix ([99, 1024, 147, 99, 1024, 147]) and passes with it.an_as_tie_keeps_the_first_pair_by_name: pins the tie rule (b9wins overalphaandzetaon the sameAS). It passes before and after.cargo test --release,cargo clippy --all-targets -- -D warningsandcargo fmt --checkare clean on macOS.Not checked
char, which is signed on x86. rustar compares unsigned bytes. SAM does not allow such names, so I did not change this.ASof -999 or lower: STAR startsbScoreat -999, so such a run keeps a stale pair. rustar starts at the minimum. Not matched.🤖 Generated with Claude Code