Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle unmarked duplicates with mate MQ = 0 in Mutect2 #5734

Merged
merged 3 commits into from
Mar 1, 2019
Merged

Conversation

davidbenjamin
Copy link
Contributor

@LeeTL1220 This is for Sachet's artifact, ready to review pending the results on Firecloud looking good.

@codecov-io
Copy link

codecov-io commented Feb 27, 2019

Codecov Report

Merging #5734 into master will increase coverage by 0.01%.
The diff coverage is 94.118%.

@@              Coverage Diff               @@
##              master     #5734      +/-   ##
==============================================
+ Coverage     87.061%   87.071%   +0.01%     
- Complexity     31880     31902      +22     
==============================================
  Files           1940      1940              
  Lines         146804    146832      +28     
  Branches       16234     16238       +4     
==============================================
+ Hits          127809    127848      +39     
+ Misses         13073     13064       -9     
+ Partials        5922      5920       -2
Impacted Files Coverage Δ Complexity Δ
...hellbender/tools/walkers/mutect/Mutect2Engine.java 89.091% <94.118%> (+0.741%) 89 <13> (+12) ⬆️
...aplotypecaller/HaplotypeCallerIntegrationTest.java 87.931% <0%> (-0.175%) 87% <0%> (+2%)
...pecaller/readthreading/ReadThreadingAssembler.java 68.077% <0%> (ø) 52% <0%> (ø) ⬇️
...ithwaterman/SmithWatermanIntelAlignerUnitTest.java 60% <0%> (ø) 2% <0%> (ø) ⬇️
...lotypecaller/readthreading/ReadThreadingGraph.java 89.216% <0%> (+0.49%) 160% <0%> (+1%) ⬆️
...walkers/haplotypecaller/AssemblyRegionTrimmer.java 66.364% <0%> (+0.909%) 21% <0%> (ø) ⬇️
...tools/walkers/mutect/SomaticLikelihoodsEngine.java 88.889% <0%> (+1.084%) 18% <0%> (+4%) ⬆️
...nstitute/hellbender/utils/clipping/ClippingOp.java 86.452% <0%> (+2.258%) 93% <0%> (ø) ⬇️
...utils/smithwaterman/SmithWatermanIntelAligner.java 80% <0%> (+30%) 3% <0%> (+2%) ⬆️

@LeeTL1220
Copy link
Contributor

@davidbenjamin Results on FC looked really good. Reviewed manually.

Copy link
Contributor

@LeeTL1220 LeeTL1220 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change and one question. I will approve on the assumption that you can address without my need to re-review.

assemblyRegion.removeAll(readStubs);
}

private void removeUnmarkedDuplicates(final AssemblyRegion assemblyRegion) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it considers:

  • SVs: This code checks whether the mates are clustered, even if on a different contig from the supporting reads of the variant.
  • Duplicate start and end positions of supporting reads.

Please tell me if I have misinterpreted. If I have not misinterpreted, then no action required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right.

@@ -1150,4 +1150,20 @@ private static String keyForVariant( final VariantContext variant ) {
return String.format("%s:%d-%d %s, %s", variant.getContig(), variant.getStart(), variant.getEnd(), variant.getReference(),
variant.getAlternateAlleles().stream().map(Allele::getDisplayString).sorted().collect(Collectors.toList()));
}

@Test
public void testSachet() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename to something about missed duplicates? And if you want to thank Sachet in a comment, that should be okay.

@LeeTL1220
Copy link
Contributor

@davidbenjamin Back to you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants