-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: implement workflow evaluation. #292
Conversation
This commit implements workflow evaluation in `wdl-engine` and in the `wdl run` command. Workflow evaluation works by taking the workflow evaluation graph and "splitting" the graph into subgraphs at each entry into a conditional and scatter statement. For conditional statements, their subgraphs are only evaluated if the conditional expression evaluates to true. For scatter statements, their subgraphs are evaluated for each element in the scatter array. Workflow evaluation is currently single-threaded. Progress is made on the evaluation graph as much as possible until no more progress can be made, at which point evaluation will wait for outstanding tasks to complete. Currently local task execution is limited to the amount of available parallelism supported on the host system; in the future, we'll want that to be configurable.
The test error is:
Looking into it (probably yet another line ending difference issue). |
Not a bug; the WDL test source isn't quoting the path in the task's command, causing the backslashes in the path on Windows to be interpreted as escape sequences. The task was failing in a piped command (also wasn't setting pipefail), which is why the task succeeded, but the I'll push up a fix momentarily. |
The fix is to properly quote the file that is being line counted in the task so that the backslashes in the path aren't misinterpreted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
Co-authored-by: Clay McLeod <3411613+claymcleod@users.noreply.github.com>
Co-authored-by: Andrew Frantz <andrew.frantz@stjude.org>
# The expression in this string results in an error (calling `select_first` on an array | ||
# containing no non-`None` values) and so the placeholder evaluates to the empty string and | ||
# `s` evalutes to: "Foo is " | ||
String s = "Foo is ~{select_first([foo])}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd expect this to fail at runtime, but because the failure is nested inside a placeholder it instead just creates an empty string? That's an interesting side effect that could lead to hard to find bugs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, if a failure occurs due to an evaluation of a None
, the placeholder doesn't fail but evaluates to empty.
If an expression within a placeholder evaluates to None, and either causes the entire placeholder to evaluate to None or causes an error, then the placeholder is replaced by the empty string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was aware of the of the first part but not the error clause. That seems problematic to me.
This PR implements workflow evaluation in
wdl-engine
and in thewdl run
command.Workflow evaluation works by taking the workflow evaluation graph and "splitting" the graph into subgraphs at each entry into a conditional and scatter statement.
For conditional statements, their subgraphs are only evaluated if the conditional expression evaluates to true.
For scatter statements, their subgraphs are evaluated for each element in the scatter array.
Workflow evaluation is currently single-threaded. Progress is made on the evaluation graph as much as possible until no more progress can be made, at which point evaluation will wait for outstanding tasks to complete.
Currently local task execution is limited to the amount of available parallelism supported on the host system; in the future, we'll want that to be configurable.
Before submitting this PR, please make sure:
changes (when appropriate).
CHANGELOG.md
(see"keep a changelog" for more information).