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

setting default compression level to 2 #4547

Merged
merged 1 commit into from
Mar 23, 2018

Conversation

lbergelson
Copy link
Member

I'm honestly not sure which of the various settings take precedence, so I set them all... We should remove the irrelevant ones. I assume that that includes the ones in the gatk launch script as well as the defaults in build.gradle?

@lbergelson lbergelson force-pushed the lb_set_default_compression_to_2 branch from 4a25185 to 9e32dcb Compare March 21, 2018 16:02
@lbergelson
Copy link
Member Author

@jonn-smith I forgot to change the default in the GATKConfig.java, and your test caught it. That's awesome!

@codecov-io
Copy link

codecov-io commented Mar 21, 2018

Codecov Report

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

@@               Coverage Diff               @@
##              master     #4547       +/-   ##
===============================================
+ Coverage     79.721%   79.724%   +0.003%     
  Complexity     16812     16812               
===============================================
  Files           1054      1054               
  Lines          60485     60485               
  Branches        9905      9905               
===============================================
+ Hits           48219     48221        +2     
+ Misses          8401      8399        -2     
  Partials        3865      3865
Impacted Files Coverage Δ Complexity Δ
...e/hellbender/engine/spark/SparkContextFactory.java 73.973% <0%> (+2.74%) 11% <0%> (ø) ⬇️

Copy link
Collaborator

@jonn-smith jonn-smith left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

@jonn-smith jonn-smith assigned lbergelson and unassigned jonn-smith Mar 21, 2018
Copy link
Contributor

@droazen droazen left a comment

Choose a reason for hiding this comment

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

👍 from me as well (though we may revisit if Intel's profiling results prove to be not what we expect)

@lbergelson lbergelson merged commit a13dea2 into master Mar 23, 2018
@lbergelson lbergelson deleted the lb_set_default_compression_to_2 branch March 23, 2018 18:33
cwhelan pushed a commit to cwhelan/gatk-linked-reads that referenced this pull request May 25, 2018
changing the default compression level 1 -> 2
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