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

Revert gCNV WDLs to tar calls from all samples. #5225

Merged
merged 2 commits into from
Sep 27, 2018
Merged

Conversation

samuelklee
Copy link
Contributor

@samuelklee samuelklee commented Sep 26, 2018

Closes #5217.

@vruano, I've verified that cohort and scattered-case mode WDLs run correctly on FC (to be precise, I tested the WDLs from my sl_filter branch rebased on this branch, but this branch alone should be fine as well).

However, #4397 should still be addressed at some point in the future. This can wait until we have v1 of the FC evaluation in place.

@codecov-io
Copy link

codecov-io commented Sep 26, 2018

Codecov Report

Merging #5225 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #5225   +/-   ##
=========================================
  Coverage      86.8%    86.8%           
  Complexity    29713    29713           
=========================================
  Files          1820     1820           
  Lines        137406   137406           
  Branches      15160    15160           
=========================================
  Hits         119268   119268           
  Misses        12634    12634           
  Partials       5504     5504

Copy link
Collaborator

@asmirnov239 asmirnov239 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 fixing this @slee. Just one a few lines that need need to be deleted, as they are not necessary anymore

mkdir -p CALLS_$index/SAMPLE_${sample_index}
tar xzf $gcnv_calls_tar -C CALLS_$index/SAMPLE_${sample_index}
mkdir CALLS_$index
tar xzf $gcnv_calls_tar -C CALLS_$index
cp ${dollar}{calling_configs_array[$index]} CALLS_$index/
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to pass as input or untar any of these anymore:

   Array[File] calling_configs
   Array[File] denoising_configs
   Array[File] gcnvkernel_version
   Array[File] sharded_interval_lists

Look at the diff for cnv_common_tasks.wdl f0dddbc#diff-f3a83ecdd107a7c42aee06dc41842fd2

Copy link
Contributor Author

@samuelklee samuelklee Sep 27, 2018

Choose a reason for hiding this comment

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

Thanks for catching this. I removed them from the PostprocessGermlineCNVCalls task but left them as outputs of the GermlineCNVCaller tasks. Might be useful for debugging and we will need it for when we do address #4397.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also added the counts as outputs of the scattered case workflow. Perhaps at some point we can flatten the outputs there. This is trivial for all outputs except the ploidy calls, which need to be uncompressed, concatenated, and compressed again.

@samuelklee
Copy link
Contributor Author

samuelklee commented Sep 27, 2018

Thanks, back to you @asmirnov239.

EDIT: Nevermind, I see that you approved. Merging!

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