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

[Augur wf] Potential bug with augur_clades call block conditional run_traits #633

Open
kapsakcj opened this issue Sep 27, 2024 · 4 comments

Comments

@kapsakcj
Copy link
Contributor

🐛

📝 Describe the Issue

Can someone please double check the logic here?

I think this is set incorrectly and should be if (run_traits); <do stuff below> instead of how it's set currently.

@jrotieno can you comment?

We have a user that is running HAV through Augur and despite run_traits being set to false, the augur_clades task is queued & run (due to organism_parameters sub workflow running and defining a minimal clades TSV file (that is empty except for column headers)) and the task fails due to no clades being defined in the minimal clades TSV file:

Augur_clades task output failure:

ERROR: No clades were defined in /cromwell_root/theiagen-public-files-rp/terra/augur-defaults/minimal-clades.tsv

workflow here: https://app.terra.bio/#workspaces/vrdl-billing/dataAnalysis_HAV_VRDL/job_history/fbb5974a-04fd-472e-b3e4-41f8a3b64aa7

🔁 How to Reproduce

v1.3.0

💻 Version Information

@kapsakcj
Copy link
Contributor Author

https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Young_Sandbox/job_history/55abb7a6-f17a-4992-9d11-4d9ea2d8511c

With this test workflow, we set run_traits to true and if the workflow succeeds (and skips the augur_clades task) then this helps provide evidence that the conditional logic is incorrect. Let's see!

@jrotieno
Copy link
Contributor

Yup, something needs to be fixed here.

@kapsakcj
Copy link
Contributor Author

The workflow linked above did indeed skip the augur_clades task, so I think the code change is simply removing the ! from the line I permalinked above. So if(run_traits) { ....

@kapsakcj
Copy link
Contributor Author

Noting that we need to review the documentation for this particular Augur optional input genome_length_input and could be done as part of this PR. It currently states that it is required for organisms without standards (like SC2, MPXV, etc.) but it should NOT be provided by the user for the Augur workflow.

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

No branches or pull requests

2 participants