feat: implement WDL 1.2 task evaluation. #265

peterhuene · 2024-12-09T22:08:36Z

This commit adds evaluation of WDL tasks with full 1.2 support.

It completes 1.2 expression evaluation except for accessing call values; those
will be implemented along with workflow evaluation.

It includes several big refactorings to wdl-analysis to facilitate in
evaluation, namely storing diagnostics, id, and uri in Document instead of
AnalysisResult.

Evaluation of string and command literals now properly unescape any escape
sequences.

Task evaluation tests were implemented and derived from examples taken directly
from the WDL 1.2 spec.

Storage of compound types has been changed such that empty compound types (e.g.
[], {}, object {}) do not cause an allocation.

Whitespace stripping for multiline strings and commands has been refactored to
reduce the number of allocations made.

Before submitting this PR, please make sure:

You have added a few sentences describing the PR here.
You have added yourself or the appropriate individual as the assignee.
You have added at least one relevant code reviewer to the PR.
Your code builds clean without any errors or warnings.
You have added tests (when appropriate).
You have updated the README or other documentation to account for these
changes (when appropriate).
You have added an entry to the relevant CHANGELOG.md (see
"keep a changelog" for more information).
Your commit messages follow the conventional commit style.

peterhuene · 2024-12-09T22:09:13Z

Note: I haven't tested this on Windows yet, so expect CI failures there.

I plan on pushing up more file tests before merging this.

wdl-engine/src/backend/local.rs

wdl-engine/src/eval.rs

wdl-engine/src/eval/v1/task.rs

wdl-engine/src/stdlib/quote.rs

wdl-engine/src/value.rs

wdl/src/lib.rs

claymcleod

I saw in an earlier version that you were evaluating and comparing the output of the commands explicitly too. I liked that being checked is well, is there a reason you had to move away from that?

peterhuene · 2024-12-10T15:01:25Z

The commands ended up with temp file names in them sometimes; I can look at what tempfile does for the file name and do a regex replacement on it to something consistent.

peterhuene · 2024-12-10T15:53:26Z

Whoops, need to strip more paths from the command files.

a-frantz

Haven't gone through all the tests yet. Or checked out the branch and kicked the tires of the new run command in the wdl bin, but here's my comments so far. Will come back with some more feedback later.

wdl-engine/src/backend/local.rs

wdl-engine/src/eval.rs

wdl-engine/src/eval/v1/task.rs

wdl-engine/src/value.rs

wdl-engine/tests/tasks.rs

claymcleod

Looks great.

wdl-analysis/CHANGELOG.md

This commit refactors analysis results so that an analysis document contains the identifier, URI, and the diagnostics resulting from the analysis of the document. As a consequence, the `ParseResult` enumeration was removed and merged with `AnalysisResult`.

This commit adds evaluation of WDL tasks with full 1.2 support. It completes 1.2 expression evaluation except for accessing call values; those will be implemented along with workflow evaluation. It includes several big refactorings to `wdl-analysis` to facilitate in evaluation, namely storing diagnostics, id, and uri in `Document` instead of `AnalysisResult`. Evaluation of string and command literals now properly unescape any escape sequences. Task evaluation tests were implemented and derived from examples taken directly from the WDL 1.2 spec. Storage of compound types has been changed such that empty compound types (e.g. `[]`, `{}`, `object {}`) do not cause an allocation. Whitespace stripping for multiline strings and commands has been refactored to reduce the number of allocations made.

These test cases come directly from the 1.2 WDL specification.

Use constants for task fields, requirement names, and hint names. Improve error messages. Fix typos.

Improve error messages. Don't delete the temp directory on retry (implement retry in the future). Expand on comments. Add an `--overwrite` option to `wdl run`.

Pass the task identifier through when evaluating a task. Add a test for the task variable in 1.2.

Co-authored-by: Andrew Frantz <andrew.frantz@stjude.org>

peterhuene requested review from adthrasher, claymcleod and a-frantz December 9, 2024 22:08

peterhuene self-assigned this Dec 9, 2024

peterhuene force-pushed the task-eval branch 6 times, most recently from 84317cb to 100d3ab Compare December 10, 2024 01:38