update BUSCO to v5.7.1 and small tweaks to WDL task

kapsakcj commented 3 months ago

This PR closes #345

🗑️ This dev branch should be deleted after merging to main.

:brain: Aim, Context and Functionality

Update BUSCO to the latest available version

:hammer_and_wrench: Impacted Workflows/Tasks & Changes Being Made

This will affect the behavior of the workflow(s) even if users don’t change any workflow inputs relative to the last version : Yes, BUSCO task auto-downloads their database at runtime and it is periodically updated (not sure how often but last update for enterobacteriales db was 2024-01-08

Running this workflow on different occasions could result in different results, e.g. due to use of a live database, "latest" docker image, or stochastic data processing : Yes

Impacted workflows:

All TheiaProk workflows (ILMN PE, ILMN SE, ONT, FASTA)
TheiaEuk_illumina_pe_phb

:clipboard: Workflow/Task Step Changes

🔄 Data Processing

Docker/software or software versions changed: upgraded to use a Theiagen-hosted copy of the ezlabgva (authors) docker image us-docker.pkg.dev/general-theiagen/ezlabgva/busco:v5.7.1_cv1

Databases or database versions changed: Database changes without warning

Data processing/commands changed: added -cpu option to main busco command

File processing changed: adjustments to parsing of output files; see code for details

Compute resources changed: none

➡️ Inputs

⬅️ Outputs

Added String busco_docker output to WDL task

TODO:

[x] add busco_docker output to all impacted workflows
[x] theiaprok_illumina_pe
[x] theiaprok_illumina_se
[x] theiaprok_illumina_fasta
[x] theiaprok_ont
[x] theiaeuk_illumina_pe

:test_tube: Testing

Test Dataset

Will update later, but will likely test across a diverse set of bacterial species and at least one eukaryotic pathogen (candida auris?)

✅ TheiaProk_FASTA_PHB on diverse set of bacterial pathogens: https://app.terra.bio/#workspaces/theiagen-validations/curtis-sandbox-theiagen-validations/job_history/95178a27-95c1-4dba-91d8-4f2413cad520
- Results are nearly identical to previous version of busco and we can see the new busco_docker output.
- Each sample is duplicated in this screenshot, showing previous BUSCO v5.3.2 results with the latest results. Slight discrepancies exist, likely due to slight differences in assemblies or updated BUSCO databases:

Commandline Testing with MiniWDL or Cromwell (optional)

Tested the WDL task changes locally:

2024-04-04 17:12:04.718 wdl.t:busco done
2024-04-04 17:12:04.719 miniwdl-run.CallCache call cache insert :: cache_file: "/home/curtis_kapsak/.cache/miniwdl/busco/x3vxz4e57qtwr25ajw5hutw77eakiukd/vbsofqftg2typeltmjrgqa5cvw26m27k.json"
{
  "outputs": {
    "busco.busco_report": "/home/curtis_kapsak/github/public_health_bioinformatics/20240404_170838_busco/out/busco_report/03-98DDCS_busco-summary.txt",
    "busco.busco_results": "C:99.8%[S:99.3%,D:0.5%],F:0.2%,M:0.0%,n:440",
    "busco.busco_database": "enterobacterales_odb10 (2024-01-08)",
    "busco.busco_docker": "us-docker.pkg.dev/general-theiagen/ezlabgva/busco:v5.7.1_cv1",
    "busco.busco_version": "BUSCO 5.7.1"
  },
  "dir": "/home/curtis_kapsak/github/public_health_bioinformatics/20240404_170838_busco"
}

Will test workflows in Terra after code has been updated

Terra Testing

Suggested Scenarios for Reviewer to Test

Theiagen Version Release Testing (optional)

:microscope: Final Developer Checklist

[x] The workflow/task has been tested locally and results, including file contents, are as anticipated
[x] The workflow/task has been tested on Terra and results, including file contents, are as anticipated
[x] The CI/CD has been adjusted and tests are passing (to be completed by Theiagen developer)
[x] Code changes follow the style guide

🎯 Reviewer Checklist

[ ] All impacted workflows/tasks have been tested on Terra with a different dataset than used for development
[ ] All reviewer-suggested scenarios have been tested and any additional
[ ] All changed results have been confirmed to be accurate
[ ] All workflows/tasks impacted by change/s have been tested using a standard validation dataset to ensure no unintended change of functionality
[ ] All code adheres to the style guide
[ ] MD5 sums have been updated
[ ] The PR author has addressed all comments

🗂️ Associated Documentation (to be completed by Theiagen developer)

[ ] Relevant documentation on the Public Health Resources "PHB Main" has been updated
[x] Workflow diagrams have been updated to reflect changes

kevinlibuit commented 2 months ago

@kapsakcj any hesitation in taking this out of draft state? Changes are looking pretty solid to me.

kapsakcj commented 2 months ago

I can mark it ready for review, but I haven't finished testing & reviewing outputs. Only ran TheiaProk_FASTA workflow linked above, haven't tested the other workflows yet.

I would recommend testing TheiaEuk to confirm it still works as intended for eukaryotes before merging.

cimendes commented 2 months ago

Testing time!

TheiaProk_Illumina_PE: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/258f14a5-4342-42f9-9b6e-95b373561780 ✅
TheiaProk_Illumina_SE: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/60000306-27a5-41ec-811e-1dedd7077ca3 ✅
TheiaEuk_Illumina_PE: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/3165f7fc-732c-4355-b299-2607f4a597f8 ⚠️

cimendes commented 2 months ago

@kapsakcj BUSCO keeps failing on TheiaEuk 😢 It "fails successfully" so it's hard for me to understand why it's so unhappy. I'll try to dig a bit and I shall report back!

cimendes commented 2 months ago

I just did a retry on the workflow for theiaeuk, setting the memory for 16GB -> https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/98834755-7d1c-43a3-aab3-fc0e66c53c03

kapsakcj commented 2 months ago

Testing TheiaEuk with 3 Candida auris genomes here, now that the default RAM is set to 24GB for TheiaEuk specifically: https://app.terra.bio/#workspaces/theiagen-validations/PHB_Validation_nextcladeV3testing/job_history/b03b3a98-99a5-443e-b15a-9fd879f56b6d

kapsakcj commented 2 months ago

BUSCO ran successfully (without memory failure) with the new default of 24GB.

I think we are good to merge?

theiagen / public_health_bioinformatics