[TheiaProk Workflows] Add Kraken2 as optional module #286

cimendes · 2023-12-22T14:46:16Z

Closes #279

🛠️ Changes Being Made

This PR adds Kraken2 as an optional module in the TheiaProk suite of workflows (Illumina PE and SE). Once PR #240 gets merged, Kraken2 will be possible to be added as an optional module for TheiaProk_ONT

Impacted Workflows/Tasks

TheiaProk PE
TheiaProk SE

🧠 Context and Rationale

Requested to facilitate contamination detection. A database must be provided to the module.

📋 Workflow/Task Steps

Kraken2 was integrated on to the Trim SE and Trim PE sub-workflows

Inputs

New optional inputs:

Boolean call_kraken = false
File? kraken_db

Outputs

New optional outputs for TheiaProk:

String? kraken2_version = read_QC_trim.kraken_version
File? kraken2_report = read_QC_trim.kraken_report
String? kraken2_docker = read_QC_trim.kraken_docker

Impacted Outputs

🧪 Testing

~~Underway~~

Locally

Not tested locally

Terra

Kraken2 Database used: gs://theiagen-public-files-rp/terra/theiaprok-files/k2_standard_08gb_20230605.tar.gz

TheiaProk_Illumina_PE: 20 Bacterial samples from various species: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/1edcf12a-c0d3-42a4-88ec-d7854765b9c2 (1 expected failure due to SRA Lite FASTQ file format)
-TheiaProk_Illumina_SE: 23 Kleb pneumoniae: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Mendes_Sandbox/job_history/5bb4c1e4-5977-45e9-973a-c6ae3264b3dd (6 expected failures due to missing read data)

Scenarios for Reviewer to Test

Check for new functionality:

TheiaProk SE
TheiaProk PE

Check that no conflicts were created:

TheiaCoV PE
TheiaCoV SE

🔬 Quality checks

Pull Request (PR) checklist:

Include a description of what is in this pull request in this message.
The workflow/task has been tested locally and on Terra
The CI/CD has been adjusted and tests are passing
Everything follows the style guide

…tion from cromwell JSON

kevinlibuit · 2024-01-04T20:17:52Z

workflows/utilities/wf_read_QC_trim_pe.wdl

@@ -107,6 +109,17 @@ workflow read_QC_trim_pe {
        midas_db = midas_db
    }
  }
+  if ("~{workflow_series}" == "theiaprok") {


Nice! I like how this is done by taking advantage of our workflow series tag

It was a suggestion by @michellescribner ! :D

kevinlibuit · 2024-01-04T20:20:40Z

tasks/taxon_id/task_kraken2.wdl

@@ -52,6 +52,7 @@ task kraken2_theiacov {
    Float percent_sc2 = read_float("PERCENT_SC2")
    String percent_target_org = read_string("PERCENT_TARGET_ORG")
    String? kraken_target_org = target_org
+    String docker = "us-docker.pkg.dev/general-theiagen/staphb/kraken2:2.0.8-beta_hv"


Ah, great catch. Didn't realize we weren't capturing this already.

kevinlibuit · 2024-01-04T20:25:15Z

workflows/utilities/wf_read_QC_trim_se.wdl

+        input:
+          samplename = samplename,
+          read1 = read1_raw,
+          kraken2_db = select_first([kraken_db])


These things always throw me off. Just to make sure I have this right: since the kraken2_db workflow variable is optional, but the task variable is obligate, we have to use this select_first() function, otherwise the WDL syntax check will prevent it from importing into Terra--is that correct?

yes! it even fails our internal CI with miniwdl check!

michellescribner · 2024-01-05T21:36:33Z

Launched TheiaProk tests:
PE: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Scribner_Sandbox/job_history/e669dc0d-d130-48f3-868c-3907c1ee431a
SE: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Scribner_Sandbox/job_history/6cfa9dcf-6189-42ad-be32-6335a1795cdb

Launched TheiaCoV function tests:
TheiaCoV_Illumina_PE_PHB: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Scribner_Sandbox/job_history/7749f154-531d-43d9-ae11-3ae8275dc6ed
TheiaCoV_Illumina_SE_PHB: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Scribner_Sandbox/job_history/6676a91d-d806-4e15-9c53-36500e86e0ed

Also gave TheiaProk_ONT_PHB a function test:
https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Scribner_Sandbox/job_history/8669d358-ad9d-48fc-912f-ee67b23a9488

kevinlibuit

Code changes look great and I appreciate the functional tests on Terra from @michellescribner. All my comments are really just comments and questions rather than any need for modifications. @cimendes this PR is ready for merge!

kevinlibuit

Ah, wait a minute. Just realizing now that @michellescribner's tests with TheiaProk properly produced the optional Kraken output, but that the TheiaCoV workflows are failling.

Dismissing initial approval based on theiacov failures

kevinlibuit · 2024-01-07T18:39:02Z

Seemingly an issue with a required input and not changes made in this PR. Relaunching some of @michellescribner's tests with inputs defined to verify:

kevinlibuit

Looks good after re-running those TheiaCoV samples with all required inputs: https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Scribner_Sandbox/job_history

michellescribner

Great job Ines!

michellescribner · 2024-01-08T18:26:56Z

Last minute I also launched a quick function test for TheiaProk without implementing the kraken module https://app.terra.bio/#workspaces/theiagen-validations/Theiagen_Scribner_Sandbox/job_history/2f7adb95-9df7-4ca6-9ffe-ee0efbd04711

cimendes · 2024-01-09T10:09:35Z

The documentation has been updated to reflect this merge!

cimendes added 4 commits December 21, 2023 18:21

first atempt, not integrated yet into theiaprok

5a8fc66

add kraken2 to theiaprok outputs

9e816d5

add to terra tools taxon_tables

e6ebf69

add kraken2 to theiaprok_se

4d3524c

cimendes requested a review from michellescribner December 22, 2023 14:46

cimendes and others added 9 commits December 22, 2023 17:36

update md5sum

779c6c9

Merge branch 'main' into im-kraken2-to-theiaprok

d8d28a9

update CI

6c554ef

update CI

d16e69d

make cromwell tests happy

fd1dcec

coherce kraken2 report to String to handle optional output, remove op…

b32611c

…tion from cromwell JSON

make kraken_report output to be string to allow empty string as output

c981518

update md5sum

78f36de

update CI

ade1356

cimendes requested a review from kevinlibuit January 3, 2024 16:43

cimendes added 2 commits January 3, 2024 17:15

pass workflow_series to read_qc_trim subworkflow

0a785f5

update CI

ea3aaec

cimendes marked this pull request as ready for review January 3, 2024 17:50

kevinlibuit reviewed Jan 4, 2024

View reviewed changes

kevinlibuit previously approved these changes Jan 7, 2024

View reviewed changes

kevinlibuit reviewed Jan 7, 2024

View reviewed changes

kevinlibuit approved these changes Jan 7, 2024

View reviewed changes

Merge branch 'main' into im-kraken2-to-theiaprok

2e321e0

michellescribner approved these changes Jan 8, 2024

View reviewed changes

update CI

d19fd28

cimendes merged commit b3503f0 into main Jan 8, 2024
17 checks passed

cimendes deleted the im-kraken2-to-theiaprok branch January 8, 2024 18:47

This was referenced Feb 2, 2024

[Kraken2_Standalone] Make task compatible with uncompressed FASTQ files #331

Merged

[TheiaMeta] Kraken2 inputs duplicated due to PR #286 #337

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TheiaProk Workflows] Add Kraken2 as optional module #286

[TheiaProk Workflows] Add Kraken2 as optional module #286

cimendes commented Dec 22, 2023 •

edited

Loading

kevinlibuit Jan 4, 2024

cimendes Jan 8, 2024

kevinlibuit Jan 4, 2024

kevinlibuit Jan 4, 2024

cimendes Jan 8, 2024

michellescribner commented Jan 5, 2024 •

edited

Loading

kevinlibuit left a comment

kevinlibuit left a comment

kevinlibuit commented Jan 7, 2024

kevinlibuit left a comment

michellescribner left a comment

michellescribner commented Jan 8, 2024

cimendes commented Jan 9, 2024

[TheiaProk Workflows] Add Kraken2 as optional module #286

[TheiaProk Workflows] Add Kraken2 as optional module #286

Conversation

cimendes commented Dec 22, 2023 • edited Loading

🛠️ Changes Being Made

Impacted Workflows/Tasks

🧠 Context and Rationale

📋 Workflow/Task Steps

Inputs

Outputs

Impacted Outputs

🧪 Testing

Locally

Terra

Scenarios for Reviewer to Test

🔬 Quality checks

kevinlibuit Jan 4, 2024

Choose a reason for hiding this comment

cimendes Jan 8, 2024

Choose a reason for hiding this comment

kevinlibuit Jan 4, 2024

Choose a reason for hiding this comment

kevinlibuit Jan 4, 2024

Choose a reason for hiding this comment

cimendes Jan 8, 2024

Choose a reason for hiding this comment

michellescribner commented Jan 5, 2024 • edited Loading

kevinlibuit left a comment

Choose a reason for hiding this comment

kevinlibuit left a comment

Choose a reason for hiding this comment

kevinlibuit commented Jan 7, 2024

kevinlibuit left a comment

Choose a reason for hiding this comment

michellescribner left a comment

Choose a reason for hiding this comment

michellescribner commented Jan 8, 2024

cimendes commented Jan 9, 2024

cimendes commented Dec 22, 2023 •

edited

Loading

michellescribner commented Jan 5, 2024 •

edited

Loading