Adding `FuncotateSegments` as an option to the Somatic CNV WDL. #5967

LeeTL1220 · 2019-05-24T20:45:41Z

Adding FuncotateSegments as an option to the Somatic CNV WDL.

Added automated WDL test to find obvious failures.

LeeTL1220 · 2019-05-24T20:47:39Z

@mwalker174 You should review the docs, etc. As well as the WDL itself. And also, FC deployment will need to know about this before the next release, since the imports changed.

codecov · 2019-05-24T21:20:02Z

Codecov Report

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

@@               Coverage Diff               @@
##              master     #5967       +/-   ##
===============================================
- Coverage     86.929%   85.845%   -1.084%     
+ Complexity     32714     32489      -225     
===============================================
  Files           2013      2016        +3     
  Lines         151268    151460      +192     
  Branches       16604     16628       +24     
===============================================
- Hits          131496    130021     -1475     
- Misses         13718     15435     +1717     
+ Partials        6054      6004       -50

Impacted Files	Coverage Δ	Complexity Δ
...s/GermlineContigPloidyModelArgumentCollection.java	`0% <0%> (-100%)`	`0% <0%> (-4%)`
...mlineContigPloidyHybridADVIArgumentCollection.java	`0% <0%> (-100%)`	`0% <0%> (-3%)`
...ments/GermlineCNVHybridADVIArgumentCollection.java	`0% <0%> (-100%)`	`0% <0%> (-3%)`
...r/arguments/GermlineCallingArgumentCollection.java	`0% <0%> (-100%)`	`0% <0%> (-4%)`
...tools/examples/ExampleStreamingPythonExecutor.java	`0% <0%> (-96.667%)`	`0% <0%> (-8%)`
...ents/GermlineDenoisingModelArgumentCollection.java	`0% <0%> (-96.552%)`	`0% <0%> (-4%)`
...number/arguments/HybridADVIArgumentCollection.java	`0% <0%> (-94.915%)`	`0% <0%> (-3%)`
...der/utils/python/PythonScriptExecutorUnitTest.java	`3.846% <0%> (-94.231%)`	`1% <0%> (-11%)`
...der/tools/walkers/vqsr/CNNVariantPipelineTest.java	`5.882% <0%> (-94.118%)`	`2% <0%> (-6%)`
.../tools/copynumber/PostprocessGermlineCNVCalls.java	`0% <0%> (-93.023%)`	`0% <0%> (-26%)`
... and 45 more

LeeTL1220 · 2019-06-03T15:36:43Z

@jonn-smith @mwalker174 Could one or both of you review, please?

jonn-smith · 2019-06-10T17:10:50Z

scripts/cnv_wdl/somatic/cnv_somatic_funcotate_seg_workflow.wdl

@@ -0,0 +1,151 @@
+workflow CNVFuncotateSegmentsWorkflow {


This should have some comments at the top to help with input arguments / valid types ala the Funcotator WDL:

# Run Funcotator on a set of called variants from Mutect 2. # # Description of inputs: # # Required: # String gatk_docker - GATK Docker image in which to run # File ref_fasta - Reference FASTA file. # File ref_fasta_index - Reference FASTA file index. # File ref_fasta_dict - Reference FASTA file sequence dictionary. # File variant_vcf_to_funcotate - Variant Context File (VCF) containing the variants to annotate. # File variant_vcf_to_funcotate_index - Index file corresponding to the input Variant Context File (VCF) containing the variants to annotate. # String reference_version - Version of the reference being used. Either `hg19` or `hg38`. # String output_file_name - Path to desired output file. # String output_format - Output file format (either VCF or MAF). # Boolean compress - Whether to compress the resulting output file. # Boolean use_gnomad - If true, will enable the gnomAD data sources in the data source tar.gz, if they exist. # # Optional: # interval_list - Intervals to be used for traversal. If specified will only traverse the given intervals. # data_sources_tar_gz - Path to tar.gz containing the data sources for Funcotator to create annotations. # transcript_selection_mode - Method of detailed transcript selection. This will select the transcript for detailed annotation (either `CANONICAL` or `BEST_EFFECT`). # transcript_selection_list - Set of transcript IDs to use for annotation to override selected transcript. # annotation_defaults - Annotations to include in all annotated variants if the annotation is not specified in the data sources (in the format <ANNOTATION>:<VALUE>). This will add the specified annotation to every annotated variant if it is not already present. # annotation_overrides - Override values for annotations (in the format <ANNOTATION>:<VALUE>). Replaces existing annotations of the given name with given values. # gatk4_jar_override - Override Jar file containing GATK 4.0. Use this when overriding the docker JAR or when using a backend without docker. # funcotator_extra_args - Extra command-line arguments to pass through to Funcotator. (e.g. " --exclude-field foo_field --exclude-field bar_field ") # # This WDL needs to decide whether to use the ``gatk_jar`` or ``gatk_jar_override`` for the jar location. As of cromwell-0.24, # this logic *must* go into each task. Therefore, there is a lot of duplicated code. This allows users to specify a jar file # independent of what is in the docker file. See the README.md for more info.

jonn-smith

Looks good.

A couple of questions / requests.

jonn-smith · 2019-06-10T17:14:18Z

scripts/cnv_wdl/somatic/cnv_somatic_funcotate_seg_workflow.wdl

+
+
+         echo "Removing $DATA_SOURCES_FOLDER"
+         ${removing_untared_datasources}


Why bother doing this? My understanding is that you pull the image every time, so this won't persist. Is that not true?

If you are running on-prem compute or local, you will have un-tar'ed datasources throughout your output directories. These will not get removed unless you are on the cloud.

I am open to suggestions for a better solution. Without WDL Directory support, I only know of sub-optimal choices.

No action.

jonn-smith · 2019-06-10T17:15:04Z

scripts/cnv_wdl/somatic/cnv_somatic_funcotate_seg_workflow.wdl

+             ${extra_args_arg}
+
+
+         echo "Removing $DATA_SOURCES_FOLDER"


You can remove this echo and add it to the definition of removing_untared_datasources to make it less confusing.

jonn-smith · 2019-06-10T17:39:48Z

scripts/cnv_wdl/somatic/cnv_somatic_pair_workflow.wdl

+    if (select_first([is_run_funcotator, false])) {
+        call CNVFuncotateSegments.CNVFuncotateSegmentsWorkflow as CNVFuncotateSegmentsWorkflow {
+            input:
+                 input_seg_file = CallCopyRatioSegmentsTumor.called_copy_ratio_segments,


Do you want to expose any more input arguments here as possibilities now? It seems like it may be useful to add them all up-front.

jonn-smith · 2019-06-10T17:43:31Z

scripts/cnv_wdl/somatic/cnv_somatic_funcotate_seg_workflow.wdl

+        File funcotated_seg_simple_tsv = "${basename_input_seg_file}.funcotated.tsv"
+        File funcotated_gene_list_tsv = "${basename_input_seg_file}.funcotated.tsv.gene_list.txt"
+    }
+}


Suggested change

}

}

jonn-smith · 2019-06-10T17:44:36Z

scripts/cnv_cromwell_tests/somatic/run_cnv_somatic_workflows.sh

+java -jar ${CROMWELL_JAR} run /home/travis/build/broadinstitute/gatk/scripts/cnv_wdl/somatic/cnv_somatic_pair_workflow.wdl \
+  -i cnv_somatic_pair_wes_do-gc_workflow_funcotator_mod.json
+#####


Suggested change

#####

#####

jonn-smith · 2019-06-10T17:47:07Z

scripts/cnv_cromwell_tests/somatic/cnv_somatic_pair_wes_do-gc_workflow_funcotator.json

+  "CNVSomaticPairWorkflow.normal_bam_idx": "/home/travis/build/broadinstitute/gatk/src/test/resources/large/cnv_somatic_workflows_test_files/SM-74NEG-v1-chr20-downsampled.deduplicated.cram.crai",
+  "CNVSomaticPairWorkflow.read_count_pon": "/home/travis/build/broadinstitute/gatk/src/test/resources/large/cnv_somatic_workflows_test_files/wes-do-gc.pon.hdf5",
+  "CNVSomaticPairWorkflow.ref_fasta_dict": "/home/travis/build/broadinstitute/gatk/src/test/resources/large/cnv_somatic_workflows_test_files/human_g1k_v37.chr-20.truncated.dict",
+  "CNVSomaticPairWorkflow.ref_fasta_fai": "/home/travis/build/broadinstitute/gatk/src/test/resources/large/cnv_somatic_workflows_test_files/human_g1k_v37.chr-20.truncated.fasta.fai",


As an observation, this is an odd reference to use for our data.

Just building off of existing infrastructure. We want this to run fast....

No action.

jonn-smith · 2019-06-10T17:50:27Z

scripts/cnv_cromwell_tests/somatic/cnv_somatic_pair_wes_do-gc_workflow_funcotator.json

@@ -0,0 +1,15 @@
+{


Is one test JSON enough? It doesn't have all the arguments in it that are supported, so I'm wondering if it makes sense to explicitly put them in (or have multiple test cases).

This is generally blocked by an inability to easily examine the outputs of a given cromwell run. This is captured in #6013

No action.

mwalker174

Looks solid, just a few minor comments.

mwalker174 · 2019-06-07T15:52:25Z

scripts/cnv_wdl/somatic/cnv_somatic_pair_workflow.wdl

@@ -432,6 +451,23 @@ workflow CNVSomaticPairWorkflow {
                 preemptible_attempts = preemptible_attempts
        }
    }
+    if (select_first([is_run_funcotator, false])) {
+        call CNVFuncotateSegments.CNVFuncotateSegmentsWorkflow as CNVFuncotateSegmentsWorkflow {


Add CNVFuncotateSegmentsWorkflow.funcotated_seg_simple_tsv and CNVFuncotateSegmentsWorkflow.funcotated_gene_list_tsv to the workflow outputs

Also seem to be missing CNVOncotatorWorkflow outputs

Am I missing something? Below is from the output block in the cnv_somatic_pair_workflow

File oncotated_called_file_tumor = select_first([CNVOncotatorWorkflow.oncotated_called_file, "null"]) File oncotated_called_gene_list_file_tumor = select_first([CNVOncotatorWorkflow.oncotated_called_gene_list_file, "null"])

For Funcotator: Done.

mwalker174 · 2019-06-07T15:55:45Z

scripts/cnv_wdl/somatic/cnv_somatic_funcotate_seg_workflow.wdl

+    String funcotator_ref_version
+    File? gatk4_jar_override
+    File? funcotator_data_sources_tar_gz
+    String? transcript_selection_mode


You could add a comment listing valid values

Done in preamble as per @jonn-smith comment. No further action.

mwalker174 · 2019-06-10T18:55:51Z

scripts/cnv_wdl/somatic/cnv_somatic_funcotate_seg_workflow.wdl

+             --ref-version ${funcotator_ref_version} \
+             --output-file-format SEG \
+             -R ${ref_fasta} \
+              --segments ${input_seg_file} \


Extra space here

LeeTL1220 · 2019-06-20T19:39:24Z

@mwalker174 @jonn-smith Back to you guys.

jonn-smith

Looks fine to me.

LeeTL1220 added 2 commits May 24, 2019 11:54

Maintenance commit.

4245399

Maintenance commit.

b220126

LeeTL1220 requested review from mwalker174 and jonn-smith May 24, 2019 20:46

droazen assigned jonn-smith and mwalker174 May 29, 2019

jonn-smith reviewed Jun 10, 2019

View reviewed changes

jonn-smith requested changes Jun 10, 2019

View reviewed changes

mwalker174 approved these changes Jun 10, 2019

View reviewed changes

Answering PR comments

86cab91

jonn-smith approved these changes Jun 21, 2019

View reviewed changes

LeeTL1220 merged commit f7d6876 into master Jun 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding `FuncotateSegments` as an option to the Somatic CNV WDL. #5967

Adding `FuncotateSegments` as an option to the Somatic CNV WDL. #5967

LeeTL1220 commented May 24, 2019

LeeTL1220 commented May 24, 2019

codecov bot commented May 24, 2019 •

edited

Loading

LeeTL1220 commented Jun 3, 2019

jonn-smith Jun 10, 2019

LeeTL1220 Jun 20, 2019

jonn-smith left a comment

jonn-smith Jun 10, 2019

LeeTL1220 Jun 20, 2019

jonn-smith Jun 10, 2019

LeeTL1220 Jun 20, 2019

jonn-smith Jun 10, 2019

LeeTL1220 Jun 20, 2019

jonn-smith Jun 10, 2019

LeeTL1220 Jun 20, 2019

jonn-smith Jun 10, 2019

LeeTL1220 Jun 20, 2019

LeeTL1220 Jun 20, 2019

jonn-smith Jun 10, 2019

LeeTL1220 Jun 20, 2019

jonn-smith Jun 10, 2019

LeeTL1220 Jun 20, 2019

mwalker174 left a comment

mwalker174 Jun 7, 2019

mwalker174 Jun 7, 2019

LeeTL1220 Jun 20, 2019

mwalker174 Jun 7, 2019

LeeTL1220 Jun 20, 2019

mwalker174 Jun 10, 2019

LeeTL1220 Jun 20, 2019

LeeTL1220 commented Jun 20, 2019

jonn-smith left a comment



		echo "Removing $DATA_SOURCES_FOLDER"
		${removing_untared_datasources}

Adding FuncotateSegments as an option to the Somatic CNV WDL. #5967

Adding FuncotateSegments as an option to the Somatic CNV WDL. #5967

Conversation

LeeTL1220 commented May 24, 2019

LeeTL1220 commented May 24, 2019

codecov bot commented May 24, 2019 • edited Loading

Codecov Report

LeeTL1220 commented Jun 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonn-smith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwalker174 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeeTL1220 commented Jun 20, 2019

jonn-smith left a comment

Choose a reason for hiding this comment

Adding `FuncotateSegments` as an option to the Somatic CNV WDL. #5967

Adding `FuncotateSegments` as an option to the Somatic CNV WDL. #5967

codecov bot commented May 24, 2019 •

edited

Loading