Skip to content

Added germline CNV WDL workflows - #3071

Merged
asmirnov239 merged 1 commit into
masterfrom
as_germline_cnv_wdl
Jun 12, 2017
Merged

Added germline CNV WDL workflows#3071
asmirnov239 merged 1 commit into
masterfrom
as_germline_cnv_wdl

Conversation

@asmirnov239

Copy link
Copy Markdown
Contributor

Migration of the broadinstitute/gatk-protected#1076 PR.

@samuelklee samuelklee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a few minor comments and questions.



# Output file name depending on type of coverage
String cov_output_name = if (is_wgs && (select_first([transform, ""]) == "RAW")) then "${base_filename}.coverage.tsv.raw_cov" else "${base_filename}.coverage.tsv"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we change the filenames to ${base_filename}.coverage.raw.tsv and ${base_filename}.coverage.pcov.tsv?

@samuelklee samuelklee Jun 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, perhaps @sooheelee can comment if changing ${base_filename}.coverage.tsv to ${base_filename}.coverage.pcov.tsv when transform = "PCOV" will necessitate changes to the javadoc examples for the somatic CNV tools. If so, let's just make the ${base_filename}.coverage.tsv.raw_cov to ${base_filename}.coverage.raw.tsv change for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It appears we populate our WDL scripts repo using this python script: https://github.com/broadinstitute/wdl/blob/develop/scripts/wrappers/gatk/gatkToWdlWrapper.py. At glance, it doesn't appear to require the javadoc portion of code, which is what I remember being the original target. Let's ask @knoblett to confirm.

Since you are providing recommended workflows in WDL scripts within the repo, I think as long as these are up to date, and the documentation is up to date, the WDL repo translation is less of a concern as it lists all of the tool's arguments as task variables.

I think this is what you are asking? Or are you just letting me know of these changes? I can change these in the example commands given in the javadoc portion if needed. It's no problem.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just confirmed that the WDL repo's scripts are based on the gatkDoc generated json files, which in turn are parsed from the code portion and not the doc portion.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, I was just letting you know of the change so you could update the example command. But after discussing with @asmirnov239 we decided to hold off on the pcov change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sooheelee @knoblett I wasn't aware we had internal scripts that read the gatkDoc-generated json files. The GATK4 json is somewhat different than GATK3, and the python script referenced above appears to be dependent on GATK3. @knoblett Is anyone running (or intending to run) that script on GATK4 json ? If so we probably need to make some changes to get it to work.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@cmnbroad These are scripts that live in the WDL repo. We will want to run them at some point, yes, but it's ok if there's some tweaking needed at that time. We're leaving that for when we're past the beta and close to the general release.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vdauwera Ok great - they will definitely need some tweaking.

"/home/travis/build/broadinstitute/gatk-protected/src/test/resources/large/cnv_germline_workflows_test_files/resources/homo_sapiens_germline_CN_transition_matrix_XY_Y.tsv" ],
"CNVGermlinePanelWorkflow.num_latents": "2",
"CNVGermlinePanelWorkflow.targets": "/home/travis/build/broadinstitute/gatk-protected/src/test/resources/large/cnv_germline_workflows_test_files/inputs/ice_targets_chr20xy.tsv",
"CNVGermlinePanelWorkflow.CollectCoverage.disable_all_read_filters": "false",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this set to false only in the panel workflows?

CROMWELL_JAR="cromwell-0.26.jar"

# Panel WES
java -jar ~/${CROMWELL_JAR} run /home/travis/build/broadinstitute/gatk-protected/scripts/cnv_wdl/germline/gCNV_panel_creation_workflow.wdl gCNV_panel_creation_workflow_wes.json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's change the filenames of WDL workflows/templates to start with cnv_germline_ rather than gCNV_, to be more analogous to the somatic filenames. But you can keep "gCNV" in tasknames, etc. for now.

@asmirnov239
asmirnov239 force-pushed the as_germline_cnv_wdl branch from 269d864 to 62f052b Compare June 8, 2017 21:55
@codecov-io

codecov-io commented Jun 8, 2017

Copy link
Copy Markdown

Codecov Report

Merging #3071 into master will increase coverage by 0.005%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##              master     #3071       +/-   ##
===============================================
+ Coverage     80.129%   80.133%   +0.005%     
  Complexity     16970     16970               
===============================================
  Files           1143      1143               
  Lines          61566     61565        -1     
  Branches        9592      9592               
===============================================
+ Hits           49332     49334        +2     
+ Misses          8417      8414        -3     
  Partials        3817      3817
Impacted Files Coverage Δ Complexity Δ
.../tools/coveragemodel/CoverageModelEMWorkspace.java 59.598% <100%> (-0.037%) 146 <0> (ø)
...ols/coveragemodel/CoverageModelEMComputeBlock.java 77.617% <100%> (ø) 49 <0> (ø) ⬇️
...oadinstitute/hellbender/utils/gcs/BucketUtils.java 73.649% <0%> (+2.027%) 34% <0%> (ø) ⬇️

@asmirnov239
asmirnov239 force-pushed the as_germline_cnv_wdl branch 2 times, most recently from d72a25b to 41dbc5d Compare June 12, 2017 16:25
… added gCNV wdl files, set up travis testing of the gCNV WDL workflows, created gCNV template input files and gCNV resources directory
@asmirnov239
asmirnov239 merged commit 502fd41 into master Jun 12, 2017
@davidbenjamin
davidbenjamin deleted the as_germline_cnv_wdl branch February 24, 2021 15:04
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.

6 participants