Wdlupdate #4

bshifaw · 2018-03-07T20:54:19Z

pairedtoubam Wdl now uses a readgroup tsv file as input, also added task to compose a file containing a list of the generated ubams

removed gatk software requiremnt because repo will contain more than one wdl which may use different versions.

…ile containing a list of the generated ubams

ruchim · 2018-03-12T14:05:19Z

paired-fastq-to-unmapped-bam.inputs.json

+
+  "##_Comment3": "Disk Space",
+  "##_ConvertPairedFastQsToUnmappedBamWf.PairedFastQsToUnmappedBAM.disk_space_gb": "(optional) Int?",
+  "##_ConvertPairedFastQsToUnmappedBamWf.PairedFastQsToUnmappedBAM.machine_mem_gb": "(optional) Int?",


Listed under disk space but this is a memory parameter

…ersion into wdlupdate

ruchim

Looks good overall, just some suggestions to simplify the workflow/task definition.

ruchim · 2018-03-12T14:05:45Z

paired-fastq-to-unmapped-bam.inputs.json


+  "##_Comment3": "Memory",
+  "##_ConvertPairedFastQsToUnmappedBamWf.CreateUbamList.disk_space_gb": "(optional) Int?",


Listed under memory but this is a disk space parameter

ruchim · 2018-03-12T14:18:57Z

paired-fastq-to-unmapped-bam.wdl


  # Convert multiple pairs of input fastqs in parallel
-  scatter (readgroup in readgroup_list) {
+  scatter (i in range(length(readgroup_array))) {


I like that the original scatter syntax is less verbose:

scatter (readgroup in readgroup_list) { call PairedFastQsToUnmappedBAM { input: fastq_1 = readgroup_array[1], fastq_2 = readgroup_array[2], ... } }

We switched to using a readgroup list file instead of an array because there isn't an option for map variables in FC for the other input variables (fastq_pairs and metadata)

ruchim · 2018-03-16T17:43:16Z

paired-fastq-to-unmapped-bam.wdl

+      input:
+        unmapped_bams = PairedFastQsToUnmappedBAM.output_bam,
+        ubam_list_name = ubam_list_name,
+	    docker = docker,


adjusting spacing

ruchim · 2018-03-16T17:46:12Z

paired-fastq-to-unmapped-bam.wdl

  }
  output {
    File output_bam = "${readgroup_name}.unmapped.bam"
  }
 }

+task CreateUbamList {


Can this task be simplified by using a write_lines()

command { mv ${write_lines(unmapped_bams)} ${ubam_list_name}.unmapped_bams.list }

I also don't think it needs to explicitly specify disk/mem as the default Cromwell values should be sufficient. You could probably get away with just docker & preemptible count, or even hard-code preemptible to 3 as such a short task will likely not need more than one attempt.

runtime { docker: docker preemptible: select_first([preemptible_attempts, 3]) }

ruchim · 2018-03-16T17:59:09Z

README.md

@@ -23,9 +23,14 @@ This WDL converts paired FASTQ to uBAM and adds read group information
 #### Requirements/expectations
 - Pair-end sequencing data in FASTQ format (one file per orientation)
 - One or more read groups, one per pair of FASTQ files 
+- The wdl will accept a file containing a list of the readgroups along with their metadata as the input. The wdl expects a specifi format for the colomuns, please follow the format as provided :  


It's not important but maybe reword this sentence a bit?

This WDL requires an input TSV file such that each row corresponds to a readgroup and the columns contain metadata for each readgroup. The WDL depends on the input TSV to follow a specific format for the columns. Please use the template provided below as the header for the input TSV file:

ruchim

Most suggestions are around syntax. For the FC version of these WDLs, it would be good to have the default docker be from GCR and not DockerHub. Secondarily, I believe there's a place where the extensions need adjustment. Other than that, it looks good to me!

ruchim · 2018-07-13T11:39:34Z

bam-to-unmapped-bams.wdl

+## - Sorted Unmapped BAMs
+##
+## Cromwell version support
+## - Successfully tested on v31


ruchim · 2018-07-13T11:42:33Z

bam-to-unmapped-bams.wdl

+    call SortSam {
+      input:
+        input_bam = unmapped_bam,
+        sorted_bam_name = output_basename + ".unmapped.bam",


shouldn't need the + ".unmapped.bam" anymore (as output_basename includes that extension)

I think basename(unmapped_bam, ".coord.sorted.unmapped.bam") removes the unmapped.bam part so this line adds back without the .coord.sorted

ruchim · 2018-07-13T11:46:59Z

paired-fastq-to-unmapped-bam-fc.wdl

+    # Convert pair of FASTQs to uBAM
+    call PairedFastQsToUnmappedBAM {
+      input:
+      fastq_1 = readgroup_array[i][1],


sorry this is a small thing, but it would be nice to indent this list of inputs :)

ruchim · 2018-07-13T11:47:41Z

paired-fastq-to-unmapped-bam-fc.wdl

+    }
+  }
+
+    #Create a file with a list of the generated ubams


adjust spacing

ruchim · 2018-07-13T11:48:14Z

paired-fastq-to-unmapped-bam-fc.wdl

+    #Create a file with a list of the generated ubams
+  call CreateFoFN {
+    input:
+    array_of_files = PairedFastQsToUnmappedBAM.output_bam,


ruchim · 2018-07-13T11:51:50Z

paired-fastq-to-unmapped-bam.inputs.json

-  "ConvertPairedFastQsToUnmappedBamWf.PairedFastQsToUnmappedBAM.disk_size": 200
-}
+  "##_Comment4": "Docker",
+  "#ConvertPairedFastQsToUnmappedBamWf.gatk_docker_override": "",


Similar to the line below, possibly replace empty "" with (optional) String?

ruchim · 2018-07-13T11:53:39Z

paired-fastq-to-unmapped-bam-fc.wdl

+  String ubam_list_name = basename(readgroup_list,".list") + "unmapped.bam.list"
+
+  String? gatk_docker_override
+  String gatk_docker = select_first([gatk_docker_override, "broadinstitute/gatk:latest"])


can the default docker be the ones in gcr instead of dockerhub? Especially as this is the FireCloud version?

i've modified the ones in FC to use gcr. The idea was to have the ones in git use docker to avoid any authentication issues for users running on different platforms.

oops no i didn't , I forgot this was the FC version. thanks for catching this.

ruchim · 2018-07-13T11:54:44Z

paired-fastq-to-unmapped-bam.wdl

    }
  }

+    #Create a file with a list of the generated ubams


ruchim · 2018-07-13T11:54:52Z

paired-fastq-to-unmapped-bam.wdl

+    #Create a file with a list of the generated ubams
+  call CreateFoFN {
+    input:
+    array_of_files = PairedFastQsToUnmappedBAM.output_bam,


ruchim · 2018-07-13T11:55:11Z

paired-fastq-to-unmapped-bam.wdl

  }
  runtime {
    docker: docker
-    memory: mem_size
+    memory: select_first([machine_mem_gb,10]) + " GB"


spacing after comma

bshifaw added 8 commits February 23, 2018 18:24

added cram-to-bam. updated pairedtoubam to use gatk4

2b7b36f

Update README.md

e1346f0

removed gatk software requiremnt because repo will contain more than one wdl which may use different versions.

Update README.md

aabc36a

Wdl now uses a readgroup tsv file as input. Added task to compose a f…

d692fd2

…ile containing a list of the generated ubams

minor

e130e12

minor

68e5689

minor edits

b55836f

Merge branch 'master' into wdlupdate

9c240ec

bshifaw requested a review from vdauwera March 7, 2018 20:56

ruchim reviewed Mar 12, 2018

View reviewed changes

bshifaw added 2 commits March 13, 2018 02:12

corrected memory placement

fb16888

Merge branch 'wdlupdate' of github.com:gatk-workflows/seq-format-conv…

a20b02d

…ersion into wdlupdate

ruchim requested changes Mar 16, 2018

View reviewed changes

bshifaw removed the request for review from vdauwera June 1, 2018 19:01

bshifaw added 8 commits June 20, 2018 15:08

minor edits

ebd40a9

added bam-to-unmapped-bams wdl

6c05f3e

fixed comment number

b22d2cf

changed to use latest gatk docker

781f298

fastq to bam now uses arrays as input

c687f15

updated descriptor for paired fastq to bam

a60dfa5

updated inpute in description

f0d60fa

added a firecloud version for fastq to Ubam

bd17e47

ruchim approved these changes Jul 13, 2018

View reviewed changes

minor format changes. chaged pairedfastq2Ubam docker to gcr

3b2f731

bshifaw merged commit 8f36d69 into master Jul 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wdlupdate #4

Wdlupdate #4

bshifaw commented Mar 7, 2018 •

edited

Loading

ruchim Mar 12, 2018

ruchim left a comment

ruchim Mar 12, 2018

ruchim Mar 12, 2018

bshifaw Jun 20, 2018

ruchim Mar 16, 2018

ruchim Mar 16, 2018

ruchim Mar 16, 2018

ruchim left a comment

ruchim Jul 13, 2018

bshifaw Jul 13, 2018

ruchim Jul 13, 2018

bshifaw Jul 13, 2018

ruchim Jul 13, 2018

ruchim Jul 13, 2018

bshifaw Jul 13, 2018

ruchim Jul 13, 2018

bshifaw Jul 13, 2018

ruchim Jul 13, 2018

bshifaw Jul 13, 2018

ruchim Jul 13, 2018

bshifaw Jul 13, 2018

ruchim Jul 13, 2018

bshifaw Jul 13, 2018

bshifaw Jul 13, 2018

ruchim Jul 13, 2018

bshifaw Jul 13, 2018

ruchim Jul 13, 2018

bshifaw Jul 13, 2018

ruchim Jul 13, 2018

bshifaw Jul 13, 2018


		"##_Comment3": "Memory",
		"##_ConvertPairedFastQsToUnmappedBamWf.CreateUbamList.disk_space_gb": "(optional) Int?",

Wdlupdate #4

Wdlupdate #4

Conversation

bshifaw commented Mar 7, 2018 • edited Loading

Choose a reason for hiding this comment

ruchim 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

ruchim 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

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

bshifaw commented Mar 7, 2018 •

edited

Loading