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

Make M2 haplotype and clustered events filters smarter about germline events #8717

Merged
merged 13 commits into from
Mar 25, 2024

Conversation

davidbenjamin
Copy link
Contributor

Closes #7391

@gatk-bot
Copy link

gatk-bot commented Mar 6, 2024

Github actions tests reported job failures from actions build 8172680846
Failures in the following jobs:

Test Type JDK Job ID Logs
variantcalling 17.0.6+10 8172680846.2 logs

@gatk-bot
Copy link

gatk-bot commented Mar 7, 2024

Github actions tests reported job failures from actions build 8181065857
Failures in the following jobs:

Test Type JDK Job ID Logs
variantcalling 17.0.6+10 8181065857.2 logs
variantcalling 17.0.6+10 8181065857.2 logs

Copy link

codecov bot commented Mar 7, 2024

Codecov Report

Merging #8717 (be4b7de) into master (c97faf6) will decrease coverage by 2.767%.
The diff coverage is 87.500%.

❗ Current head be4b7de differs from pull request most recent head 2f1c832. Consider uploading reports for the commit 2f1c832 to get more accurate results

Additional details and impacted files
@@               Coverage Diff               @@
##              master     #8717       +/-   ##
===============================================
- Coverage     86.407%   83.640%   -2.767%     
+ Complexity     39882     38213     -1669     
===============================================
  Files           2371      2371               
  Lines         187596    187624       +28     
  Branches       20532     20537        +5     
===============================================
- Hits          162096    156928     -5168     
- Misses         18272     23475     +5203     
+ Partials        7228      7221        -7     
Files Coverage Δ
...alkers/mutect/filtering/ClusteredEventsFilter.java 100.000% <100.000%> (ø)
...kers/mutect/filtering/FilteredHaplotypeFilter.java 91.111% <100.000%> (-6.667%) ⬇️
...e/hellbender/utils/variant/GATKVCFHeaderLines.java 95.522% <100.000%> (ø)
...r/tools/walkers/mutect/Mutect2IntegrationTest.java 5.405% <0.000%> (-89.189%) ⬇️
.../tools/walkers/mutect/SomaticGenotypingEngine.java 83.258% <88.571%> (-10.524%) ⬇️

... and 141 files with indirect coverage changes

@ldgauthier ldgauthier self-requested a review March 7, 2024 14:16
Copy link
Contributor

@ldgauthier ldgauthier left a comment

Choose a reason for hiding this comment

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

How hard would it be to put in a regression test where you do something similar to #7391 and compare with and without --genotype-germline-sites? There's not a lot of coverage for that arg as-is. And I'm a little concerned that while the ECNT annotation changed in the SW test, there were no filtering changes in order tests.

@davidbenjamin
Copy link
Contributor Author

Added a regression test with two good variants that this PR rescues from being false negatives. Also made the sensitivity thresholds in the other integration tests higher because even without --genotype-germline-sites this PR saves a few calls.

@gatk-bot
Copy link

Github actions tests reported job failures from actions build 8349590850
Failures in the following jobs:

Test Type JDK Job ID Logs
variantcalling 17.0.6+10 8349590850.2 logs

@ldgauthier
Copy link
Contributor

I saw your "whoops" commit -- I have actually merged commits that turn off tests. James is trying to figure out if we can have codecov check the absolute number of tests since that should almost never go down. He claims you can run select tests from the data provider with IntelliJ, but it seems... cumbersome.

Copy link
Contributor

@ldgauthier ldgauthier 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 new test! The sensitivity threshold is a good find too

@gatk-bot
Copy link

Github actions tests reported job failures from actions build 8354454301
Failures in the following jobs:

Test Type JDK Job ID Logs
variantcalling 17.0.6+10 8354454301.2 logs

@gatk-bot
Copy link

Github actions tests reported job failures from actions build 8384399332
Failures in the following jobs:

Test Type JDK Job ID Logs
variantcalling 17.0.6+10 8384399332.2 logs

@davidbenjamin davidbenjamin merged commit 105b63e into master Mar 25, 2024
20 checks passed
@davidbenjamin davidbenjamin deleted the db_m2_haplotype_filter branch March 25, 2024 14:20
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.

Mutect2 genotype-germline-sites filtering discrepancy
3 participants