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

[Freyja] Update freyja to version 1.5.2, expose pathogen flag and minor update to docs #684

Merged
merged 6 commits into from
Dec 11, 2024

Conversation

cimendes
Copy link
Member

@cimendes cimendes commented Dec 6, 2024

This PR closes #655

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

🧠 Summary

This PR does the following:

  • Updates Freyja workflow suite to the lates version, v1.5.2
  • Exposes the pathogen flag, accessed via freyja_pathogen for easier support for non-SC2 pathogens
  • Does minimal updates to the documentation. A full documentation revamp is planned but I'll address it in a new PR.

There's a few quirks with the pathogen flag. If barcodes are passed, even if the organisms flag is used, it will be ignored and the provided barcodes will take precedence. the user can still use the --update-db and the pathogen flag but they behave the same way. Functionally they are the same as in they will fetch the latest barcodes from the repository automatically. For compatibility purposes the update_db option was not removed.

⚡ Impacted Workflows/Tasks

All the Freyja workflows, but all but Freyja_FASTQ_PHB had just the docker updated.

This PR may lead to different results in pre-existing outputs: Yes if update_db or freyja_pathogen is used

This PR uses an element that could cause duplicate runs to have different results: Yes if update_db or freyja_pathogen is used

🛠️ Changes

⚙️ Algorithm

The Freyja pathogen flag is now exposed as input in Freyja_FASTQ_PHB.

➡️ Inputs

New input:

  • freyja_pathogen

Altered inputs:

  • freyja_usher_barcodes is now freyja_barcodes as usher is only for SC2 and freyja is now compatible with multiple pathogens

image

⬅️ Outputs

No outputs have been changed

🧪 Testing

SC2

NON-SC2

Suggested Scenarios for Reviewer to Test

More non-SC2 organisms but it's really hard to find samples! One measles sample can be found here: https://andersen-lab.github.io/Freyja/src/wiki/freyja-measles.html

🔬 Final Developer Checklist

  • The workflow/task has been tested and results, including file contents, are as anticipated
  • The CI/CD has been adjusted and tests are passing (Theiagen developers)
  • Code changes follow the style guide
  • Documentation and/or workflow diagrams have been updated if applicable (Theiagen developers only)

🎯 Reviewer Checklist

  • All changed results have been confirmed
  • You have tested the PR appropriately (see the testing guide for more information)
  • All code adheres to the style guide
  • MD5 sums have been updated
  • The PR author has addressed all comments
  • The documentation has been updated

@cimendes cimendes changed the title [Freyja] Update freyja to version 1.5.2, expose pathogen flag and revamp docs [Freyja] Update freyja to version 1.5.2, expose pathogen flag and minor update to docs Dec 9, 2024
@cimendes cimendes marked this pull request as ready for review December 9, 2024 13:25
@cimendes cimendes requested a review from a team as a code owner December 9, 2024 13:25
Michal-Babins
Michal-Babins previously approved these changes Dec 10, 2024
Copy link
Contributor

@Michal-Babins Michal-Babins left a comment

Choose a reason for hiding this comment

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

All changes and updates look good. Couldn't find any syntax or logic snafus. Will test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Programmatically changes look good and make sense. Version for docker is bumped, freyja_pathogen is exposed as an optional string, freyja_usher_barcodes --> freyja-barcodes. --pathogen added to freyja boot command. Again freyja_usher_barcodes variable switched for more general freyja_barcodes.

@Michal-Babins
Copy link
Contributor

To echo what Ines pointed out about the update_db & pathogen flag as well as providing barcodes, just need to make a note for myself to understand the logic:

When you run Freyja, it needs reference files (barcodes and lineage information) to work. You have three ways to provide these:

  • Give Freyja your own custom barcode files
  • Use the pathogen flag
  • Use the update_db flag

If you provide you own custom barcode files, Freyja will use your files, it will ignore any other settings about which pathogen to look for since --barcodes is more heavily weighted.
If you pass the pathogen flag true this tells Freyja to download the latest reference files for that pathogen, but will NOT do anything if you provide custom barcodes.
update_db as true does exactly the same thing as the pathogen flag, but needs to maintained for legacy compatibility.
Custom barcodes always win out, and if you don't provide those, either pathogen or update_db will grab the latest files for you.

@Michal-Babins
Copy link
Contributor

Test with freyja_pathogen set to "SARS-CoV-2" passing and outputs as expected:
https://app.terra.bio/#workspaces/cdph-terrabio-taborda-manual/CDPH_Bioinformatics_Development/job_history/350f4ccc-5722-465d-a010-d2da064f8a86

Test with update_db set to true passing and outputs as expected:
https://app.terra.bio/#workspaces/cdph-terrabio-taborda-manual/CDPH_Bioinformatics_Development/job_history/85da1d8f-22e3-48fc-883b-072e6023390b

Test with both update_db set to true and pathogen set to "SARS-CoV-2"
https://app.terra.bio/#workspaces/cdph-terrabio-taborda-manual/CDPH_Bioinformatics_Development/job_history/9314c45a-fcb9-489b-b04a-84e1a7d1b187

Will test with non sars-cov-2 specific organisms next.

@Michal-Babins
Copy link
Contributor

Final test with MPXV looks good when passed with the pathogen flag. Non SARS-CoV-2 data needs to be specified with --pathogen flag or else it will default to SARS-CoV-2.
https://app.terra.bio/#workspaces/cdph-terrabio-taborda-manual/CDPH_Bioinformatics_Development/job_history/1f45e146-d8e5-462a-94f2-a466fb74c101

Copy link
Contributor

@Michal-Babins Michal-Babins left a comment

Choose a reason for hiding this comment

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

After small doc change, everything is good.

@Michal-Babins Michal-Babins merged commit e552f80 into main Dec 11, 2024
8 checks passed
@Michal-Babins Michal-Babins deleted the im-freyja-pathogen-dev branch December 11, 2024 18:53
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.

Update Freyja docker image to include --pathogen flag
2 participants