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

Passing vcf indices correctly in M2 wdls #4381

Merged
merged 2 commits into from
Feb 9, 2018
Merged

Conversation

davidbenjamin
Copy link
Contributor

This is the bug fix for @eitanbanks's pon. @LeeTL1220 can you review if tests pass? If they fail I will fix ASAP.

@codecov-io
Copy link

codecov-io commented Feb 8, 2018

Codecov Report

Merging #4381 into master will decrease coverage by 0.003%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##              master    #4381       +/-   ##
==============================================
- Coverage     79.093%   79.09%   -0.003%     
  Complexity     16664    16664               
==============================================
  Files           1050     1050               
  Lines          59698    59698               
  Branches        9756     9756               
==============================================
- Hits           47217    47215        -2     
- Misses          8691     8696        +5     
+ Partials        3790     3787        -3
Impacted Files Coverage Δ Complexity Δ
...utils/smithwaterman/SmithWatermanIntelAligner.java 50% <0%> (-30%) 1% <0%> (-2%)
...e/hellbender/engine/spark/SparkContextFactory.java 71.233% <0%> (-2.74%) 11% <0%> (ø)
...park/sv/discovery/alignment/AlignmentInterval.java 90.038% <0%> (+1.149%) 74% <0%> (+2%) ⬆️

@LeeTL1220
Copy link
Contributor

@davidbenjamin I think this affects more than the PoN.

@LeeTL1220
Copy link
Contributor

Are you sure that @eitanbanks was specifying compress of true? If it was false, then I believe we've misdiagnosed the issue. Doubly odd, since false is the default.

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. Also, please see my questions. Back to you @davidbenjamin ... If you address my comments, feel free to merge.

gatk_override = gatk_override,
gatk_docker = gatk_docker,
preemptible_attempts = preemptible_attempts,
disk_space = m2_per_scatter_size,
auth = auth
}

Float sub_vcf_size = size(M2.output_vcf, "GB")
Float sub_vcf_size = size(M2.vcf, "GB")
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer output_vcf or something more descriptive than vcf... raw_vcf?

@davidbenjamin
Copy link
Contributor Author

Are you sure that @eitanbanks was specifying compress of true? If it was false, then I believe we've misdiagnosed the issue. Doubly odd, since false is the default.

@LeeTL1220 False is and was the default for all final outputs, but the intermediate scatter vcfs were hard-coded to be compressed.

@davidbenjamin davidbenjamin merged commit dbea0da into master Feb 9, 2018
@davidbenjamin davidbenjamin deleted the db_m2_wdl_indices branch February 9, 2018 04:27
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.

3 participants