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

Restore base quality filter code that got reverted in #4895 #5123

Merged
merged 1 commit into from
Aug 31, 2018

Conversation

takutosato
Copy link
Contributor

This unintended change crept in with #4895.

@meganshand @davidbenjamin

@takutosato
Copy link
Contributor Author

@meganshand please review!

@codecov-io
Copy link

codecov-io commented Aug 20, 2018

Codecov Report

Merging #5123 into master will decrease coverage by 0.068%.
The diff coverage is 95%.

@@               Coverage Diff               @@
##              master     #5123       +/-   ##
===============================================
- Coverage     86.735%   86.668%   -0.068%     
+ Complexity     29312     29049      -263     
===============================================
  Files           1810      1809        -1     
  Lines         135549    134690      -859     
  Branches       15031     14934       -97     
===============================================
- Hits          117569    116733      -836     
+ Misses         12566     12545       -21     
+ Partials        5414      5412        -2
Impacted Files Coverage Δ Complexity Δ
...dorientation/CollectF1R2CountsIntegrationTest.java 100% <100%> (ø) 13 <1> (-3) ⬇️
...r/tools/walkers/mutect/Mutect2IntegrationTest.java 92.515% <100%> (+0.658%) 61 <1> (+1) ⬆️
...r/tools/walkers/mutect/Mutect2FilteringEngine.java 84.375% <66.667%> (+0.164%) 50 <0> (ø) ⬇️
...ellbender/tools/walkers/mutect/M2TestingUtils.java 91.304% <91.304%> (ø) 6 <6> (?)
...iscoverFromLocalAssemblyContigAlignmentsSpark.java 72.193% <0%> (-18.334%) 9% <0%> (-15%)
...ery/inference/SimpleNovelAdjacencyInterpreter.java 80.882% <0%> (-8.824%) 11% <0%> (-1%)
...institute/hellbender/utils/NucleotideUnitTest.java 90.698% <0%> (-4.795%) 34% <0%> (-91%)
...ls/ExtractOriginalAlignmentRecordsByNameSpark.java 86.364% <0%> (-4.545%) 9% <0%> (-1%)
...nalAlignmentRecordsByNameSparkIntegrationTest.java 86.667% <0%> (-3.958%) 7% <0%> (-2%)
...ute/hellbender/tools/walkers/vqsr/TrainingSet.java 73.333% <0%> (-3.333%) 4% <0%> (-1%)
... and 66 more

@takutosato
Copy link
Contributor Author

@meganshand I added a very specific test to check for this bug that I introduced. I confirmed that the test fails before this change and passes after.

Copy link
Contributor

@meganshand meganshand left a comment

Choose a reason for hiding this comment

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

Thanks for the test!

@@ -201,8 +201,6 @@ private File createSyntheticSam(final int refDepth, final int altDepth) throws I

// create a sample list
final int chromosomeIndex = 0;
final String sampleName = "samthreetree";
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to make these changes? They look unrelated to the other test.

…uplicated code in tests surrounding Mutect2.
@takutosato takutosato merged commit 9fb2a65 into master Aug 31, 2018
@sbamin
Copy link

sbamin commented Sep 29, 2018

HI @takutosato,

if (baseQualityByAllele != null && baseQualityByAllele[indexOfMaxTumorLod + 1] < MTFAC.minMedianBaseQuality) {
filterResult.addFilter(GATKVCFConstants.MEDIAN_BASE_QUALITY_FILTER_NAME);
}

Looks like this could have impacted one or more of mutect2 somatic filtering (doc-11136). I believe bug was related to an argument, --min-median-base-quality, i.e., it was either ignoring this argument or (hope) taking a default value of 20 in Mutect2 and FilterMutectCalls tools.

  • Is that the correct interpretation?
  • Would this mandate running mutect2 from scratch or only FilterMutectCalls and subsequent filtration steps?

I am looking through our mutect2 filtered calls and filtering rules applied. Among PASS calls, so far, I don't see false positive with respect to FORMAT/MBQ < 20. Among filtered out calls, I don't have calls failing due to single filter but I need to look more into raw calls. Anyways, it would be of help to get your comments on possible impact of this bug in gatk version < 4.0.9.0 (I used 4.0.8.1).

@davidbenjamin davidbenjamin deleted the ts_restore_bq_filter branch February 24, 2021 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants