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

Stop needless copying of inputs #10

Merged
merged 5 commits into from
Jul 30, 2021
Merged

Stop needless copying of inputs #10

merged 5 commits into from
Jul 30, 2021

Conversation

aofarrel
Copy link
Owner

@aofarrel aofarrel commented Jul 26, 2021

This task takes in an array of truth files and an array of test files. Truth files are assumed to match the basename of test files. When a truth file is found to match a test file in basename, they are md5'd. If that fails, an Rscript checks if they pass all.equal() within a user-defined tolerance which defaults to 1.0e-8. It should go without saying that the Rscript isn't going to function correctly if passed a non-rdata file.

In a typical checker workflow, this task would be used to test multiple rdata outputs of a workflow's task against multiple truth files from a Google cloud bucket. But for the purposes of testing, both truth and test files are being downloaded from the cloud.

The testing confuguration is metamouse.wdl, which runs this workflow three times. In one of those tests, it is designed to pass. In the other two, it is designed to fail -- once because the exact flag is true, causing the file that that doesn't perfectly match to trigger a failure, and once because the deviation between two files is outside the value of tolerance. These are functionally equivalent but algorithmically different as the former does not call an rscript but the latter does.

@aofarrel
Copy link
Owner Author

The import in metamouse.wdl should be pointing to this branch to take up the changes on this branch, of course, but it already passes/fails when expected on Terra so I went ahead and reverted it to point at main again so I won't forget to do that after meging.

Copy link

@DailyDreaming DailyDreaming left a comment

Choose a reason for hiding this comment

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

This looks good to me.

for i in ~{sep=' ' truth}
do
truth_basename="$(basename -- ${i})"

# We assume the test file and truth file have the same basename
# Due to how WDL input work, they have a different absolute path

Choose a reason for hiding this comment

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

  • inputs

Copy link
Owner Author

Choose a reason for hiding this comment

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

This sort of thing keeps happening. I code in Sublime Text, which can spell check in comments, but it doesn't seem to recognize WDL comments as comments ("toggle comment" doesn't work even with the Broad's WDL syntax highlighter active and it seems most people just have issues with the keystroke but I'm using the menu). Still, there is a grammar check package out there, so if I can get it recognize comment scope, I should be able to stop this from happening...

Before I go down the force-sublime-to-recognize-a-comment rabbithole, do you know if there's something like lint for spelling/grammar comments? That might be easier than trying to wrangle Sublime. I've had issues with Package Control anyway.

Choose a reason for hiding this comment

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

Only the blinding mental flare of the English language wrought into a shape that was never meant to exist.

@aofarrel aofarrel merged commit d4d8bcf into main Jul 30, 2021
@aofarrel aofarrel deleted the arraycheck-stop-copying branch July 30, 2021 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants