M2 modifies base quals instead of discarding overlapping read pairs #5794
Conversation
This file contains 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
LeeTL1220
approved these changes
Mar 13, 2019
| @@ -139,14 +141,13 @@ public static void finalizeRegion(final AssemblyRegion region, | |||
| * Clean up reads/bases that overlap within read pairs | |||
| * | |||
| * @param reads the list of reads to consider | |||
There was a problem hiding this comment.
Can you describe the rest of the parameters?
There was a problem hiding this comment.
(and what can be null, etc etc)
And when making it public, please check for nulls and erroneous value ranges.
| @@ -48,6 +47,8 @@ | |||
| public static final String MEDIAN_AUTOSOMAL_COVERAGE_LONG_NAME = "median-autosomal-coverage"; | |||
| public static final String MITOCHONDRIA_MODE_LONG_NAME = "mitochondria-mode"; | |||
| public static final String CALLABLE_DEPTH_LONG_NAME = "callable-depth"; | |||
| public static final String PCR_SNV_QUAL_LONG_NAME = "pcr-snv-qual"; | |||
| public static final String PCR_INDEL_QUAL_LONG_NAME = "pcr-inel-qual"; | |||
| public int initialPCRErrorQual = 40; | ||
|
|
||
| @Argument(fullName = PCR_SNV_QUAL_LONG_NAME, optional = true, doc = "Phred-scaled PCR SNV qual for overlapping fragments") | ||
| public int PCRSnvQual = 40; |
| public int PCRSnvQual = 40; | ||
|
|
||
| @Argument(fullName = PCR_INDEL_QUAL_LONG_NAME, optional = true, doc = "Phred-scaled PCR SNV qual for overlapping fragments") | ||
| public int PCRIndelQual = 40; |
| @@ -237,6 +236,9 @@ public void writeHeader(final VariantContextWriter vcfWriter, final Set<VCFHeade | |||
|
|
|||
| final AssemblyRegion regionForGenotyping = assemblyResult.getRegionForGenotyping(); | |||
| removeReadStubs(regionForGenotyping); | |||
| AssemblyBasedCallerUtils.cleanOverlappingReadPairs(regionForGenotyping.getReads(), samplesList, header, | |||
There was a problem hiding this comment.
Put a comment on why divide by 2.
| @@ -433,7 +435,7 @@ public boolean emitReferenceConfidence() { | |||
| * @return a list of variant contexts (can be empty) to emit for this ref region | |||
| */ | |||
| private List<VariantContext> referenceModelForNoVariation(final AssemblyRegion region) { | |||
| AssemblyBasedCallerUtils.finalizeRegion(region, false, true, (byte)9, header, samplesList, ! MTAC.doNotCorrectOverlappingBaseQualities); //take off soft clips and low Q tails before we calculate likelihoods | |||
| AssemblyBasedCallerUtils.finalizeRegion(region, false, true, (byte)9, header, samplesList, false); //take off soft clips and low Q tails before we calculate likelihoods | |||
There was a problem hiding this comment.
Put a comment on why we do not need to correct here.
| Utils.nonNull(clippedSecondRead); | ||
| Utils.validateArg(clippedFirstRead.getName().equals(clippedSecondRead.getName()), () -> | ||
| "attempting to merge two reads with different names " + clippedFirstRead + " and " + clippedSecondRead); | ||
| public static void adjustQualsOfOverlappingPairedFragments(final List<GATKRead> pair, final boolean setConflictingToZero, |
There was a problem hiding this comment.
Put acceptable values and whether null is allowed in the doc.
Codecov Report
@@ Coverage Diff @@
## master #5794 +/- ##
===============================================
+ Coverage 87.002% 87.066% +0.064%
- Complexity 32103 32297 +194
===============================================
Files 1974 1975 +1
Lines 147214 148125 +911
Branches 16215 16360 +145
===============================================
+ Hits 128079 128966 +887
- Misses 13231 13248 +17
- Partials 5904 5911 +7
|
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.
Closes #4959. Closes #4958. Closes #4933.
Improves indel sensitivity in particular without hurting precision.
@LeeTL1220 Can you review this one?