Fix Empty Mean Coverage for Flu HA and NA Assemblies

jrotieno commented 4 months ago

This PR closes #361.

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

:brain: Aim, Context and Functionality

This PR fixes an issue with empty mean coverage estimates for influenza HA and Na assemblies.

: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

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, but not due to this change

:clipboard: Workflow/Task Step Changes

🔄 Data Processing

Docker/software or software versions changed: No

Databases or database versions changed: No

Data processing/commands changed: Yes, to correctly capture BAM files used to calculate assembly coverage

File processing changed: No

Compute resources changed: No

➡️ Inputs

No changes

⬅️ Outputs

No changes

:test_tube: Testing

Test Dataset

Tested using a set of 11 influenza sample reads, including one that is simulated data.

Commandline Testing with MiniWDL or Cromwell (optional)

Tested the irma task locally and worked as expected.

Terra Testing

https://app.terra.bio/#workspaces/cdph-terrabio-taborda-manual/Global_tree_testing/job_history/ffe75b13-4dde-4c51-af39-bd5215058b0e

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

[x] All impacted workflows/tasks have been tested on Terra with a different dataset than used for development
[x] All reviewer-suggested scenarios have been tested and any additional
[x] All changed results have been confirmed to be accurate
[x] All workflows/tasks impacted by change/s have been tested using a standard validation dataset to ensure no unintended change of functionality
[x] All code adheres to the style guide
[x] MD5 sums have been updated
[x] 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
[ ] Workflow diagrams have been updated to reflect changes

cimendes commented 4 months ago

@jrotieno Two samples failed on your test dataset that I believe are related to this PR! The irma error is as follows:

Sample_01_HA.fasta Sample_01_NA.fasta mv: 'Sample_01_HA.bam' and 'Sample_01_HA.bam' are the same file mv: 'Sample_01_NA.bam' and 'Sample_01_NA.bam' are the same file

Can you take a look?

I've launched a second test at https://app.terra.bio/#workspaces/theiagen-validations/PHB_Validation_PR_310/job_history/7e6d16c7-bb81-4a6c-aa02-408d1335427f on 21 FLU samples

cimendes commented 4 months ago

Good news! And bad news!

In some samples, the outputs look as expected: (sample_id, assembly_length, assembly_mean_coverage)