Incorrect parsing of wdl

Hello Terra Team,

 

I noticed strange behavior in the method configuration. It suddenly got populated with a large number of default parameters (see screenshot):

 

However, those are not optional parameters in the workflow but rather hardcoded runtime parameters:

Why would they suddenly show up in the method configuration as optional ones?

The pipeline is called getzlab/CGA_WES_Characterization_Pipeline_v0.1_Dec2018/2 and you can review configuration in the workspace: broad-fc-getzlab-workflows/CGA_WES_Characterization_OpenAccess

Thank you for your help,

Luda

Comments

16 comments

  • Comment author
    Sushma Chaluvadi

    Hi Luda,

    It looks as though the #DEFAULT VALUES that are listed are appearing in the Method Configuration even though they have default values assigned to them whereas previous to the WDL 1.0 release, the variables that were visible were just the ones that were optional and defined as `String?'. I am going to check on this to confirm and get back to you.

    Sushma

    0
  • Comment author
    Adam Nichols

    If you enter a value of "2" for default_cpu in the method configuration, it should get picked up and replace the default value of "1" when the WDL runs.

    I believe we may have previously failed to expose this behavior, which would explain the unexpected change.

     

    0
  • Comment author
    Liudmila Elagina

    I tried this approach:

    https://github.com/openwdl/wdl/blob/master/versions/1.0/SPEC.md#task-inputs

    to remove those variables from showing up in the method configurations but got this error when trying to create a new method on Firecloud:

    ```

    Error: Invalid WDL: ERROR: Unexpected symbol (line 758, col 5) when parsing ‘_gen4’. Expected rbrace, got input. input { ^ $task = :task :identifier :lbrace $_gen3 $_gen4 :rbrace -> Task( name=$1, declarations=$3, sections=$4 )

    ```

     

    0
  • Comment author
    Adam Nichols

    You probably need a "version 1.0" declaration at the top.

    Upgrading from draft-2 to 1.0 is a non-trivial change and I do recommend familiarizing yourself with the spec before embarking.

    I will also point out that draft-2 WDLs cannot be imported into 1.0 or vice-versa.

    0
  • Comment author
    Adam Nichols

    Also, I would not expect the change in display of method configuration to have any functional impact, so you could simply proceed using draft-2 with the knowledge that the display has changed.

    0
  • Comment author
    Liudmila Elagina

    Since I do not have "version 1.0" in my wdl why Firecloud parses my wdl script with wdl version 1.0?

    0
  • Comment author
    Adam Nichols

    The application automatically detects the WDL version based on the version header (no header for draft-2, version 1.0 for 1.0).

    You cannot use features from WDL 1.0 in a draft-2 WDL, like the input { } block.

    Note that the link you provided is to the WDL 1.0 spec.

    0
  • Comment author
    Liudmila Elagina

    I understand that.

    So draft-2 is the previous version of WDL that ignored default values as inputs and Version 1.0 recognizes them as optional parameters?

    Since I do not have any header in wdl for version, shouldn't it ignore those default values as before? What changed?

    0
  • Comment author
    Adam Nichols

    Based on what I can see from here, an older implementation of draft-2 in FireCloud erroneously excluded non-optional task inputs for which a default was provided.

    Is your workflow impaired from running, or are you simply concerned about the unexpected change?

    0
  • Comment author
    Sushma Chaluvadi

    Would you be able to share your WDL with us so we can take a closer look? You can send it directly to

    Terra-support@broadinstitute.zendesk.com.

    0
  • Comment author
    Liudmila Elagina

    So as I understand there is a newer version of draft-2 on Firecloud which is separate from WDL 1.0? There were no warnings of any upcoming changes to WDL. 

    This pipeline is not impaired by this particular issue but it is very concerning as there are seem to be few unexpected behaviors such as posted here (I encountered this similar issue with one sample optional array being passed as a file, not an array):

    https://support.terra.bio/hc/en-us/community/posts/360048188912-Issue-interpreting-array-inputs-to-workflow 

    The pipeline is called getzlab/CGA_WES_Characterization_Pipeline_v0.1_Dec2018/2 and you can review configuration in the workspace: broad-fc-getzlab-workflows/CGA_WES_Characterization_OpenAccess

    0
  • Comment author
    Adam Nichols

    I can't review it, because I do not have access to that workspace.

    0
  • Comment author
    Liudmila Elagina

    Apparently, I can not share this workspace but the method is public in method repository with configurations:

    getzlab/CGA_WES_Characterization_Pipeline_v0.1_Dec2018

    0
  • Comment author
    Adam Nichols

    I see it now, thank you!

    I ran the WDL through womtool on the command line and the results are the same as in the UI:

    womtool inputs CGA_WES_Characterization_Pipeline_v0.1_Dec2018.2.wdl

    includes the input default_downsampleToCoverage:

    "CGA_Production_Analysis_Workflow.Mutect1_Task.default_downsampleToCoverage": "String (optional, default = \"99999\")"

    This supports my theory that the change you see in the UI is actually fixing a bug. It would appear that the bug stuck around for so long users became accustomed to it.

    Hope this helps.

     

    0
  • Comment author
    Liudmila Elagina

    Thank you! This is very helpful. 

    0
  • Comment author
    Adam Nichols

    You are welcome, glad to hear it. This is a really complicated domain where small things matter, and you are right to question the change.

    0

Please sign in to leave a comment.