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

Fixed an optimization in KBestHaplotypeFinder that caused some bestPaths to be lost while still preserving some of the optimization. #5952

Merged
merged 1 commit into from
May 21, 2019

Conversation

jamesemery
Copy link
Collaborator

@davidbenjamin Was familiarizing myself with KBestHaplotypeFinder and decided to take a crack at this issue.

I have no idea how much of a performance hit this will end up being at extreme sites. At worst it involves adding more paths into the priority queues than existed before which could slow down the whole search algorithm.

Fixes #5907

…ths to be lost while still preserving some of the optimization.
@codecov
Copy link

codecov bot commented May 20, 2019

Codecov Report

Merging #5952 into master will increase coverage by 0.002%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##              master     #5952       +/-   ##
===============================================
+ Coverage     86.821%   86.823%   +0.002%     
- Complexity     32349     32356        +7     
===============================================
  Files           1993      1993               
  Lines         149474    149499       +25     
  Branches       16521     16521               
===============================================
+ Hits          129775    129799       +24     
- Misses         13675     13676        +1     
  Partials        6024      6024
Impacted Files Coverage Δ Complexity Δ
...s/haplotypecaller/graphs/KBestHaplotypeFinder.java 95.455% <100%> (ø) 23 <0> (ø) ⬇️
...ypecaller/graphs/KBestHaplotypeFinderUnitTest.java 98.547% <100%> (+0.094%) 81 <8> (+8) ⬆️
...nder/utils/runtime/StreamingProcessController.java 67.773% <0%> (-0.474%) 33% <0%> (-1%)

Copy link
Contributor

@davidbenjamin davidbenjamin left a comment

Choose a reason for hiding this comment

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

Nice work @jamesemery! The effect on performance is going to be negligible, and the effect on correctness is obviously beneficial. 👍

@jamesemery jamesemery merged commit 5a5c38e into master May 21, 2019
@jamesemery jamesemery deleted the je_fixBrokenOptimizationInKBestHaplotypeFinder branch May 21, 2019 14:04
@mehrzads
Copy link

Thank you @jamesemery! It looks great.

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.

Best haplotype finder functionality is changed
3 participants