Repository navigation
Store PNEXT and TLEN as differences from what the record predicts - #113
Merged
Merged
Conversation
PNEXT and TLEN were stored as SAM has them, although most of what they hold is in other columns: the mate is usually a few hundred bases from POS, and TLEN follows from the two positions, the strands and the spans. On chr20 of NA12878 at 30x, 17.7 million records, they took 57 of the 563 MB of the file. The record now stores pnext_delta, PNEXT minus POS when the mate is on the record's reference and PNEXT otherwise, and tlen_delta, TLEN minus PredictTLEN(). The prediction is the distance between the 5' ends of the pair as BWA and Picard compute it, with the mate's span taken to be the record's own, so it needs no tag. Both are differences modulo 2^32, so every value comes back exactly. RAMMateOnSameRef() decides whether the mate is on the same reference for the converters and for RAMFile::MateOnSameRef(), so writing and reading cannot disagree. SetMate() replaces SetPNEXT() and SetTLEN(), and GetPNEXT() and GetTLEN() take that answer. The columns are renamed, so a program that reads them directly does not mistake them for the SAM values; CONTRIBUTING says how to get those. The format version becomes 6. On that chr20 tlen_delta is 0 for 95% of the records. The file goes from 563.1 to 531.0 MB (-5.7%) from SAM and from 562.9 to 530.7 MB from BAM, and `ramdump -h` gives the same bytes as before for both. Full and region dumps, and conversion from SAM on 4 threads and from BAM on 2 million records, take the same time within the noise of alternating runs. flagstat and idxstats are unchanged.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #113 +/- ##
===========================================
+ Coverage 88.47% 88.64% +0.16%
===========================================
Files 25 25
Lines 2968 3029 +61
Branches 980 1004 +24
===========================================
+ Hits 2626 2685 +59
Misses 185 185
- Partials 157 159 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
clang-tidy review says "All clean, LGTM! 👍" |
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
2 participants
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.
PNEXT and TLEN were stored as SAM has them, although most of what they
hold is in other columns: the mate is usually a few hundred bases from
POS, and TLEN follows from the two positions, the strands and the
spans. On chr20 of NA12878 at 30x, 17.7 million records, they took 57
of the 563 MB of the file.
The record now stores pnext_delta, PNEXT minus POS when the mate is on
the record's reference and PNEXT otherwise, and tlen_delta, TLEN minus
PredictTLEN(). The prediction is the distance between the 5' ends of
the pair as BWA and Picard compute it, with the mate's span taken to be
the record's own, so it needs no tag. Both are differences modulo 2^32,
so every value comes back exactly. RAMMateOnSameRef() decides whether
the mate is on the same reference for the converters and for
RAMFile::MateOnSameRef(), so writing and reading cannot disagree.
SetMate() replaces SetPNEXT() and SetTLEN(), and GetPNEXT() and
GetTLEN() take that answer. The columns are renamed, so a program that
reads them directly does not mistake them for the SAM values;
CONTRIBUTING says how to get those. The format version becomes 6.
On that chr20 tlen_delta is 0 for 95% of the records. The file goes
from 563.1 to 531.0 MB (-5.7%) from SAM and from 562.9 to 530.7 MB
from BAM, and
ramdump -hgives the same bytes as before for both.Full and region dumps, and conversion from SAM on 4 threads and from
BAM on 2 million records, take the same time within the noise of
alternating runs. flagstat and idxstats are unchanged.