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

Workflow evaluation graphs do not take into account workflow inputs #356

Closed
peterhuene opened this issue Mar 18, 2025 · 2 comments · Fixed by #360
Closed

Workflow evaluation graphs do not take into account workflow inputs #356

peterhuene opened this issue Mar 18, 2025 · 2 comments · Fixed by #360
Assignees
Labels
bug Something isn't working spec compliance Behavior is not to the WDL specification

Comments

@peterhuene
Copy link
Collaborator

peterhuene commented Mar 18, 2025

I found this example from the WDL 1.0 spec:

version 1.1

task my_task {
    input {
        Int i
    }

    command <<<>>>

    output {
        Int out = i
    }
}

workflow foo {
  input {
    Int x = 10
    Int y = t1.out
  }

  call my_task as t1 { input: i = x }
  call my_task as t2 { input: i = y }
}

If you were to call foo without a value for y, t2 would depend on t1 (via y) and would have to be executed in series.

If you were to call foo with a value for y, t1 and t2 could be executed in parallel as there is no dependency edge between them.

The issue in wdl-engine is that we currently build the evaluation graph based solely on the workflow statement and always introduce dependency edges from any of the input default expressions.

Instead, this process should take into account any inputs provided to the workflow and skip adding dependency edges from the default expressions if a value was provided.

@peterhuene peterhuene added bug Something isn't working spec compliance Behavior is not to the WDL specification labels Mar 18, 2025
@peterhuene
Copy link
Collaborator Author

peterhuene commented Mar 18, 2025

The tricky part will be testing this with an assertion that the tasks ran in parallel, as workflow evaluation is a bit of black box in that regard.

I think perhaps a unit test on WorkflowEvaluationGraph where we check that an edge exists in the graph when not given the input vs. the edge does not exist when given the input will suffice.

@dead8309
Copy link
Contributor

dead8309 commented Mar 18, 2025

Hi @peterhuene can you please assign me this issue, as per our conversation on slack. I've made the necessary changes. I'm currently writing unit test for this.

Sending a patch in a few minutes. Thanks!

dead8309 added a commit to dead8309/wdl that referenced this issue Mar 18, 2025
dead8309 added a commit to dead8309/wdl that referenced this issue Mar 18, 2025
- Add sample test from stjude-rust-labs#356
- fix typo
- remove debugging logs
- update changelog
dead8309 added a commit to dead8309/wdl that referenced this issue Mar 19, 2025
- Add sample test from stjude-rust-labs#356
- fix typo
- remove debugging logs
- update changelog
- format files
- remove unncecessary to_string() calls
dead8309 added a commit to dead8309/wdl that referenced this issue Mar 19, 2025
- Add sample test from stjude-rust-labs#356
- fix typo
- remove debugging logs
- update changelog
- format files
- remove unncecessary to_string() calls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working spec compliance Behavior is not to the WDL specification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants