Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write a specification for unsatisfied task inputs. #359

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Keep the changelog pleasant to read in the text editor:

version 2.0.0
---------------------------

+ Write a specification for unsatisfied task inputs and nested optional inputs.
[PR 359](https://github.com/openwdl/wdl/pull/359) by @rhpvorderman

+ Removes string interpolator options and adds an engine function for joining arrays of strings.
[PR 229](https://github.com/openwdl/wdl/pull/229) and [PR 368](https://github.com/openwdl/wdl/pull/366)
by @EvanTheB and @illusional.
Expand Down
77 changes: 64 additions & 13 deletions versions/development/SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -2705,12 +2705,42 @@ In this example, `i`, and `f` are inputs to this task even though `i` is not dir

## Computing Workflow Inputs

Workflows have inputs that must be satisfied to run them, just like tasks. Inputs to the workflow are provided as a key/value map where the key is of the form `workflow_name.input_name`.
Workflows have inputs that must be satisfied to run them, just like tasks.
Inputs to the workflow are provided as a key/value map where the key is of the
form `workflow_name.input_name`.

* A call has its inputs supplied when called by a workflow.
* Example: `call my_task { input: my_task_input=... }`
* All required call inputs which do not have defaults should be filled by the
calling workflow.
* A workflow is allowed not to specify optional inputs in a call's input block.
In this case, the inputs bubble up to become a nested input to the workflow
instead.
* Example: an unsupplied input might have the fully-qualified name
`my_workflow.my_task.my_task_input`.
* If that workflow is used as a subworkflow, the input is allowed to bubble up
again with a further-qualified name.
* Example: my_outer_workflow.my_workflow.my_task.my_task_input.
* There is currently no way to supply a bubbled-up input in an outer workflow's
call block.
Copy link

@pieterlukasse pieterlukasse May 11, 2020

Choose a reason for hiding this comment

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

Not sure I understand this point. Does it contradict the previous one? I.e. the previous one seems to introduce an option, but this point seems to state it is not possible. Can you please clarify?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first point is about the input json (or whatever other input format)

The outer workflow's call block is in the WDL code itself. When you call a workflow from inside a workflow. In that area there is currently no way to supply nested inputs.

* Example: the following inputs would not be valid for a call to a subworkflow
`{ inputs: my_task.my_task_input=... }`
* By default an engine only allows inputs that are specified in the input
section of the top-level workflow.
* If a workflow is suitable for use with nested inputs it should be explicitly
stated. This is done by setting `allowNestedInputs` to `true` in the meta
Copy link
Collaborator

Choose a reason for hiding this comment

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

My problem with this is that things like allowNestedInputs don't really belong in meta - there's just no where else to put them. I am going to open a proposal to add a hints section to workflow with the same semantics as it has in task. I think things like this more appropriately belong in hints than in meta.

Copy link
Member

Choose a reason for hiding this comment

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

I think when designing the hints section, I ultimately planned on having one in the workflow as well. however it felt like a more natural transition to start with it in the task. In the lont term this is probably a good move. If we add the hints in 1.1, I feel like 2.0 can propogate the hints to the workflow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree! Meta was always a bit of a "let's put it where it doesn't hurt" manoeuver.

Not having this feature proved to be a major hindrance in getting actual work done. But at least it is implemented in cromwell now, so it is at least possible to get work done until this feature lands.

If the hints proposal is landed I will rewrite this PR.

section of the workflow. For example:

* If a workflow is to be used as a sub-workflow it must ensure that all of the inputs to its calls are satisfied.
* If a workflow will only ever be submitted as a top-level workflow, it may optionally leave its tasks' inputs unsatisfied. This then forces the engine to additionally supply those inputs at run time. In this case, the inputs' names must be qualified in the inputs as `workflow_name.task_name.input_name`.
```WDL
meta {
allowNestedInputs: true
}
```

Any declaration that appears outside the `input` section is considered an intermediate value and **not** a workflow input. Any declaration can always be moved inside the `input` block to make it overridable.
Any declaration that appears outside the `input` section is considered an
intermediate value and **not** a workflow input.
Any declaration can always be moved inside the `input` block to make it
overridable.

Consider the following workflow:

Expand All @@ -2722,26 +2752,32 @@ task t1 {
}

command {
./script --action=${s} -x${x}
./script --action=~{s} -x~{x}
}
output {
Int count = read_int(stdout())
}
runtime {
docker: "openwdl/examplescript:v1.1.3"
}
}

task t2 {
input {
String s
Int t
Int? t
Int x
}

command {
./script2 --action=${s} -x${x} --other=${t}
./script2 --action=~{s} -x~{x} ~{"--other=" + t}
}
output {
Int count = read_int(stdout())
}
runtime {
docker: "openwdl/examplescript2:v1.0.1"
}
}

task t3 {
Expand All @@ -2751,27 +2787,39 @@ task t3 {
}

command {
python -c "print(${y} + 1)"
python -c "print(~{y} + 1)"
}
output {
Int incr = read_int(stdout())
}
runtime {
docker: "python:3.7-slim"
}
}

workflow wf {
input {
Int int_val
Array[Int] my_ints
File ref_file
String t1s
String t2s
}
meta {
allowNestedInputs: true
}

String not_an_input = "hello"

call t1 {
input: x = int_val
input:
x = int_val,
s = t1s
}
call t2 {
input: x = int_val, t=t1.count
input:
x = t1.count,
s = t2s
}
scatter(i in my_ints) {
call t3 {
Expand All @@ -2783,13 +2831,16 @@ workflow wf {

The inputs to `wf` would be:

* `wf.t1.s` as a `String`
* `wf.t2.s` as a `String`
* `wf.t1s` as a `String`
* `wf.t2s` as a `String`
* `wf.int_val` as an `Int`
* `wf.my_ints` as an `Array[Int]`
* `wf.ref_file` as a `File`
* `wf.t2.t` as an `Int?`

Note that because some call inputs are left unsatisfied, this workflow could not be used as a sub-workflow. To fix that, additional workflow inputs could be added to pass-through `t1.s` and `t2.s`.
Note that the optional `t` input for task `t2` is left unsatisfied, this
option can be passed as `wf.t2.t` because `allowNestedInputs` is set to true
if it were set to `false` this input would not be available.

## Specifying Workflow Inputs

Expand Down