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

Ngs qc qa workflow #6

Merged
merged 17 commits into from
Oct 17, 2023
Merged

Ngs qc qa workflow #6

merged 17 commits into from
Oct 17, 2023

Conversation

rroutsong
Copy link
Collaborator

Workflow for FASTQ QC/QA pipeline including fastqc before and after trimming, fastp trimming, and fastq_screen contamination detection.

Copy link
Contributor

@skchronicles skchronicles left a comment

Choose a reason for hiding this comment

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

Everything looks good, thank you for all your hard work!

Copy link
Contributor

Choose a reason for hiding this comment

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

In the next PR, let's rename the --pretend | -p option to --dry-run | -n.

Copy link
Contributor

Choose a reason for hiding this comment

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

At some point, we are going to need a way to dynamically create this file on the fly. The current paths point to Biowulf HPC staff installations.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah that is partially the reason I chose to embed this conf file into the docker instead of use the one on the disk on biowulf. My original thought was to use common mount points, like /mnt/kaiju/... /mnt/kraken2... etc

Copy link
Contributor

Choose a reason for hiding this comment

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

In the next PR, can you set latency-wait to 120 seconds?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the next PR, can we move to pulling threads information from a config file?

adapters = get_adapter_opts,
container: "docker://rroutsong/dmux_ngsqc:0.0.1"
threads: 4
resources: mem_mb = 8192
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also pull mem_mb from a config file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With some of these commands, I ran separately through sbatch to get a feel for the resource requirements through the biowulf job dashboard, but I have not run everything thing. My plan here was to collect everything in a config once I had resolved the reqs for each rule.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to prefix the commands with conda run -n ngsqc? Can move to having the conda env activate in the container (would require building it)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there is an ep.sh that should activate ngsqc on run, so it's either preface it with that or use the conda run. I believe that the singularity exec that snakemake uses overwrites the defined entrypoint for the image. The issue is activating an environment on initialization, so far nothing I have tried has worked to be able selectively activate a defined conda environment on image startup.

@skchronicles skchronicles merged commit 04c46d2 into main Oct 17, 2023
1 check passed
@rroutsong rroutsong deleted the ngs_qc_qa_workflow branch November 17, 2023 16:39
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.

4 participants