Fixed a missing special case in MarkDuplicates ReadsKey code #4899
Conversation
… to assert the results match picard
Codecov Report
@@ Coverage Diff @@
## master #4899 +/- ##
===============================================
+ Coverage 80.453% 80.631% +0.178%
- Complexity 17837 18309 +472
===============================================
Files 1092 1092
Lines 64231 65642 +1411
Branches 10348 10735 +387
===============================================
+ Hits 51676 52928 +1252
- Misses 8504 8601 +97
- Partials 4051 4113 +62 |
| int read1ReferenceIndex = ReadUtils.getReferenceIndex(read1,header); | ||
| int read2ReferenceIndex = ReadUtils.getReferenceIndex(read2,header); | ||
|
|
||
| if( read1ReferenceIndex != read2ReferenceIndex ? read1ReferenceIndex < read2ReferenceIndex : read1UnclippedStart <= read2UnclippedStart ){ |
There was a problem hiding this comment.
Wouldn't this would be easier to read as
if( read1ReferenceIndex <= read2ReferenceIndex && read1UnclippedStart <= read2UnclippedStart) {
unless I missed something there that makes them not equivalent
There was a problem hiding this comment.
No actually that is not equivalent, if read2RefIndex > read1RefIndex BUT it starts earlier on its contig that statement would be false when it should not be
There was a problem hiding this comment.
Oh, heh, you're write. I didn't think that through.
| isRead1ReverseStrand = first.isReverseStrand(); | ||
| isRead2ReverseStrand = second.isReverseStrand(); | ||
|
|
||
| // Keep track of the orientation of read1 and read2 as it is important for optical duplicate marking | ||
| wasFlipped = second.isFirstOfPair(); //TODO be sure this is still correct... |
| firstStartPosition = first.getAssignedStart(); | ||
|
|
||
| // Ensuring that the orientation is consistent for reads starting at the same place there is no propper order |
There was a problem hiding this comment.
I think the comment in picard is clearer
There was a problem hiding this comment.
// if the two read ends are in the same position, pointing in opposite directions,
// the orientation is undefined and the procedure above
// will depend on the order of the reads in the file.
// To avoid this, we set it explicitly (to FR):
| int read1ReferenceIndex = ReadUtils.getReferenceIndex(read1,header); | ||
| int read2ReferenceIndex = ReadUtils.getReferenceIndex(read2,header); | ||
|
|
||
| if( read1ReferenceIndex != read2ReferenceIndex ? read1ReferenceIndex < read2ReferenceIndex : read1UnclippedStart <= read2UnclippedStart ){ |
There was a problem hiding this comment.
There should be a test for this edge case that we were missing.
|
@lbergelson back to you |
No description provided.