test: yeast-tier differential in a minute, and a SAM agreement tool - #253
Open
BenjaminDEMAILLE wants to merge 1 commit into
Open
test: yeast-tier differential in a minute, and a SAM agreement tool#253BenjaminDEMAILLE wants to merge 1 commit into
BenjaminDEMAILLE wants to merge 1 commit into
Conversation
CONTRIBUTING.md's differential wants the whole ERR12389696 run, which is why "measure it on the yeast tier first" has been easy to skip. This fetches a byte range instead: the first ~120 MB of each mate is 10 000 pairs, the reads are ordered so the mates still pair up, and the whole thing takes about a minute including both index builds. test/sam_agreement.py is the comparison itself, usable on any two SAM files. It reports position agreement and NH agreement separately on purpose: a tie broken differently moves the first and not the second, while a real regression usually moves both, so one number cannot tell them apart. Measured on main at 10 000 pairs: 7860 uniquely mapped against STAR's 7861, 98.48% of mates at the same chromosome, position and CIGAR, 99.96% with the same NH. Written while investigating #31, where it is what showed that a candidate fix regressed faithfulness rather than improving it.
This was referenced Aug 27, 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
1 participant
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.
Why
CONTRIBUTING.md's differential wants the whole ERR12389696 run. That is the right benchmark and the wrong thing to reach for when checking whether a stitcher change moved faithfulness, which is why "measure it on the yeast tier first" has been easy to skip.test/yeast_tier.shfetches a byte range instead: the first ~120 MB of each mate is 10 000 pairs, the reads are ordered so the mates still pair up, and the whole run takes about a minute including both index builds.Two numbers, not one
test/sam_agreement.pyreports position agreement and NH agreement separately, and that separation is the point: a tie broken differently from STAR moves the first and leaves the second alone, while a genuine regression usually moves both. Collapsing them into one percentage hides which of the two just happened.It works on any two SAM files, so it is also usable against the nf-core fixture or a full run.
Where it came from
Investigating #31. I found the mechanism (STAR drops a transcript whose blocks are a subset of another in the same window and which scores lower; this codebase gates that check on the two transcripts having the same number of exons, and the offending variants have one fewer), implemented STAR's version, and this script is what showed the result: the tail disappears and faithfulness gets worse. Details on the issue, with the numbers. The change is not in this PR — only the tooling that measured it.
🤖 Generated with Claude Code