Add a task for SnpSift filter and update snpEff #340
Conversation
…en run on dnanexus
rhpvorderman
left a comment
There was a problem hiding this comment.
Looks good to me. I did found some nitpicks.
bcftools.wdl
Outdated
| String? regions | ||
| Boolean splitMultiallelicSites = false | ||
|
|
||
| String memory = "64GiB" |
There was a problem hiding this comment.
That seems quite a lot. Does it read everything into memory?
There was a problem hiding this comment.
Yeah, it doesn't need that much, but I needed dnanexus to select a larger VM type with enough scratch space to fit the output file in. I couldn't find a way to do that differently :/ I'll have another look, there should be a better way...
There was a problem hiding this comment.
There are disk space requirements in WDL I believe. We never used them because disk space is not exactly a problem on our storage. Or is that something for later versions?
There was a problem hiding this comment.
That was added in version 1.1, I believe. I did find an example in the dxcompiler docs using the disks runtime attribute, so I've just copied that now.
| mkdir -p "$(dirname ~{outputPath})" | ||
| bcftools norm \ | ||
| -o ~{outputPath} \ | ||
| -O ~{true="z" false="v" compressed} \ |
There was a problem hiding this comment.
There is an opportunity here to set it by default to z1:
-O, --output-type u|b|v|z[0-9] u/b: un/compressed BCF, v/z: un/compressed VCF, 0-9: compression level [v]
That way it is always quite fast.
snpsift.wdl
Outdated
|
|
||
| command { | ||
| set -e | ||
| ls ~{vcf} ~{vcfIndex} |
There was a problem hiding this comment.
Seems like an old debug command got in the task.
snpeff.wdl
Outdated
|
|
||
| command { | ||
| set -e | ||
| ls ~{vcf} ~{vcfIndex} |
There was a problem hiding this comment.
Is this a debug command?
bcftools.wdl
Outdated
|
|
||
| command { | ||
| set -e | ||
| ls ~{inputFile} ~{inputFileIndex} |
There was a problem hiding this comment.
Is this a debug command?
There was a problem hiding this comment.
No, this is a workaround for dxcompiler. It will only localize files mentioned in the command.
There was a problem hiding this comment.
Oh, fun, galaxy has the same issue with pulsar.
There was a problem hiding this comment.
🤦♂️ . There is probably a reason for this design decision? Or is it an oversight? Should we file a bug?
There was a problem hiding this comment.
There's an issue open for in on their repo, but they claimed that: "WDL spec did not specify (it specifies the WDL syntax rather than the execution environment behavior)". Which is simply not true...
Checklist
parameter_metawas added/updated (if required).