fix(solo): port STAR's actual CellRanger4 clip rules - #241
Open
BenjaminDEMAILLE wants to merge 1 commit into
Open
fix(solo): port STAR's actual CellRanger4 clip rules#241BenjaminDEMAILLE wants to merge 1 commit into
BenjaminDEMAILLE wants to merge 1 commit into
Conversation
Both ends of --clipAdapterType CellRanger4 were approximations of rules STAR implements exactly, so the clip lengths differed from STAR's in both directions. 5' TSO. STAR aligns the TSO against the read with Opal in overlap mode (ClipCR4.cpp: match +1, mismatch -2, gap open and extend 2) and clips endLocationTarget + 1 bases unless the alignment is too weak (ClipMate_clipChunk.cpp: S < 20, or S == 20 with L > 26, or S == 21 with L > 30). Here it was a fixed-length prefix comparison with a mismatch budget, which cannot see a TSO that starts a few bases into the read and fires on full-length matches STAR scores below the floor. Ported as an affine-gap DP: the query is 30 bases and the window is 91, so it is a small fixed cost. 3' poly-A. Ported ClipCR4::polyTail3p verbatim: +1 per A, -2 otherwise, remember the longest prefix scoring at least 70% of its length, stop once the score falls more than 27 behind, and require a final score of 20. The previous rule trimmed only a literal run of A of length >= 8, so a tail with one sequencing error kept about half its length in the alignment. Also adds test/cr4_clip_diff.py, a synthetic STAR-vs-rustar differential for this flag combination that runs in seconds instead of needing the 10x mouse chr19 dataset, and ignores the local .claude/ and *.code-workspace scratch files that keep landing in `git add -A`.
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.
--clipAdapterType CellRanger4was approximating two rules that STAR implements exactly, so its clip lengths differed from STAR's in both directions.5' TSO
STAR aligns the TSO against the read with Opal in overlap mode (
ClipCR4.cpp: match +1, mismatch −2, gap open and extend 2), then clipsendLocationTarget + 1bases unless the alignment is too weak (ClipMate_clipChunk.cpp):Here it was a fixed-length prefix comparison with a mismatch budget. That rule cannot see a TSO starting a few bases into the read, and it fires on full-length matches whose score STAR rejects. Ported as an affine-gap DP; the query is 30 bases against a 91-base window, so the cost is small and fixed.
3' poly-A
ClipCR4::polyTail3pis a scored scan: +1 perA, −2 otherwise, remember the longest prefix scoring ≥70% of its length, stop once the score falls more than 27 behind, require a final score of 20, and never trim a read under 20 bases. The previous rule trimmed a literal run ofAof length ≥ 8, so a tail with a single sequencing error kept roughly half its length in the alignment.What this does and does not settle about #199
It does not close #199, and I want to be exact about why.
test/cr4_clip_diff.py(added here) builds a synthetic solo fixture and runs both aligners with--clipAdapterType CellRanger4 --clip5pNbases 5 --clip3pNbases 3. On it, every read comes out with identical CIGARs and rustar's POS exactly 5 higher than STAR's:Ground truth from the fixture: read
r0occupies 1-based 17421..17510, and with a 5-base 5' clip the first aligned base is 17426. The control settles it — the same reads, same clips, without--clipAdapterType CellRanger4, in plain single-end mode:Both tools agree there, and they agree with ground truth. So on this fixture it is STAR's CR4 path that reports POS at the first soft-clipped base rather than the first aligned one, while its CIGAR still says
5S.That is one direction of #199's ±5 histogram, measured on synthetic data, not on the 10x mouse chr19 dataset the issue used. The other direction is what this PR addresses (clip lengths that were not STAR's). Settling the real-data histogram needs a rerun on that dataset with this branch, which I could not do here.
Tests
Nine unit tests covering both rules, including the cases that separate them from the old approximations: a TSO starting inside the read, a full TSO with three mismatches, a five-base TSO overlap that stays below STAR's score floor, a poly-A tail carrying one error, and a short
Arun that is not a tail. Whole suite,cargo clippy --all-targets -- -D warningson a cold cache, andcargo fmt --checkare green.🤖 Generated with Claude Code