organize for metapipeline #23

alkaZeltser · 2023-11-08T22:53:49Z

Description

Updates to turn this pipeline into a useful component of metapipline.

Added a couple new processes to handle merging of target and off-target intervals
Rearranged workflow logic to more intuitively follow pipeline default functions
Added parameterization of many optional outputs
Updated README (A LOT)
Created a workflow diagram

Closes #15

Urgent things to address before official release:

Organize tool-specific reference files in appropriate /hot/ref/ location.
- See Notes on off-target coverage calculation filtered by dbSNP loci #18 for notes on dbSNP reference file formatting. /hot/user/nzeltser/pipeline-targeted-coverage/test/input/dbSNP-155_thinned_hg38.vcf.gz
- Additional non-standard ref files: genome sizes required by bedtools slop and provided with the bedtools distribution: /hot/user/nzeltser/tools/bedtools2/genomes/human.hg38.genome
Confirm standardized output directory naming. Currently all outputs fall under SAMtools-1.16.1 as this is the tool behind the main read depth calculation process - do we want to keep this?
Should the targeted sequencing dataset be converted into an official test set?
Do I need to implement checksum creation? Does not currently exist.

Testing Results

Testing set is the BZPRGPT2 PRAD targeted panel pilot cohort 2 with original target file.

Case 1: Just CollectHsMetrics
- output: /hot/software/pipeline/pipeline-targeted-coverage/Nextflow/development/unreleased/nzeltser-organize-for-metapipeline/hs-metrics-only
Case 2: run all processes that produce a final output; no intermediate files
- output: /hot/software/pipeline/pipeline-targeted-coverage/Nextflow/development/unreleased/nzeltser-organize-for-metapipeline/all-final-outputs-no-intermediates
Case 3: run all processes and output intermediate files
- output: /hot/software/pipeline/pipeline-targeted-coverage/Nextflow/development/unreleased/nzeltser-organize-for-metapipeline/all-outputs-with-intermediates

Checklist

I have read the code review guidelines and the code review best practice on GitHub check-list.
I have reviewed the Nextflow pipeline standards.
The name of the branch is meaningful and well formatted following the standards, using [AD_username (or 5 letters of AD if AD is too long)]-[brief_description_of_branch].
I have set up or verified the branch protection rule following the github standards before opening this pull request.
I have added my name to the contributors listings in the manifest block in the nextflow.config as part of this pull request, am listed
already, or do not wish to be listed. (This acknowledgement is optional.)
I have added the changes included in this pull request to the CHANGELOG.md under the next release version or unreleased, and updated the date.
I have updated the version number in the metadata.yaml and manifest block of the nextflow.config file following semver, or the version number has already been updated. (Leave it unchecked if you are unsure about new version number and discuss it with the infrastructure team in this PR.)
I have tested the pipeline on at least one A-mini sample.

yashpatel6

Added some comments:

main.nf

yashpatel6 · 2023-11-10T02:33:40Z

main.nf

+    // Calculate HsMetrics
+    if ( params.collect_metrics ) {
+
+        //Get intervalLists (only needed for collectHsMetrics)
+        //if you already have the interval list use that, otherwise, generate interval list from BedToIntervalList process


Not necessary in this PR but I'd recommend trying to put the processes under each if statement into their own workflows to simplify main.nf

module/merge_bedfiles_bedtools.nf

yashpatel6 · 2023-11-10T02:51:47Z

pipeline-targeted-coverage.drawio.svg

Also not necessarily in this PR, but I'd recommend using PlantUML to diagram for pipelines as it then allows for the diagram to be tracked as code and is easily modifiable compared to just image files that can be hard to modify and track changes for

yashpatel6 · 2023-11-10T02:57:28Z

Urgent things to address before official release:
* Organize tool-specific reference files in appropriate /hot/ref/ location.

  * See [Notes on off-target coverage calculation filtered by dbSNP loci #18](https://github.com/uclahs-cds/pipeline-targeted-coverage/discussions/18) for notes on dbSNP reference file formatting. /hot/user/nzeltser/pipeline-targeted-coverage/test/input/dbSNP-155_thinned_hg38.vcf.gz
  * Additional non-standard ref files: genome sizes required by `bedtools slop` and provided with the `bedtools` distribution: /hot/user/nzeltser/tools/bedtools2/genomes/human.hg38.genome

* Confirm standardized output directory naming. Currently all outputs fall under `SAMtools-1.16.1` as this is the tool behind the main read depth calculation process - do we want to keep this?

Generally, yes we want the outputs under the main tool of the pipeline; since this pipeline really only has one main tool for performing its main function, that should stay as is (as opposed to pipelines like call-sSNV where there are multiple main tools since multiple tools for calling sSNVs are implemented)

* Should the targeted sequencing dataset be converted into an official test set?

It should, through NFTest, become a formal test case; in terms of eventually publicizing the pipeline (either through metapipeline or on its own), consider any potential concerns about whether the targeted sequencing dataset can be public in any way

* Do I need to implement checksum creation? Does not currently exist.

I would recommend yes; we have a modularized process for it so it should be straight-forward to add - https://github.com/uclahs-cds/pipeline-Nextflow-module/tree/main/modules/PipeVal/generate-checksum

alkaZeltser · 2023-11-10T18:29:46Z

* Do I need to implement checksum creation? Does not currently exist.
I would recommend yes; we have a modularized process for it so it should be straight-forward to add - https://github.com/uclahs-cds/pipeline-Nextflow-module/tree/main/modules/PipeVal/generate-checksum

Can you share an up-to-date implementation example from an existing pipeline?

yashpatel6 · 2023-11-17T16:27:41Z

* Do I need to implement checksum creation? Does not currently exist.
I would recommend yes; we have a modularized process for it so it should be straight-forward to add - https://github.com/uclahs-cds/pipeline-Nextflow-module/tree/main/modules/PipeVal/generate-checksum
Can you share an up-to-date implementation example from an existing pipeline?

Here's an example of it that was used when the module was created and tested: https://github.com/uclahs-cds/pipeline-align-DNA/blob/8b5ba8508df4f3affd9391711190e56f8e2cf1ec/module/align_DNA_BWA_MEM2.nf#L19-L26

yashpatel6

Generally looks good! There are some things that can be cleaned up but not needed in this PR. Can you move the outputs with the checksums to the dev directory?

Anything else to add @sorelfitzgibbon ?

main.nf

sorelfitzgibbon · 2023-11-29T02:25:48Z

Anything else to add @sorelfitzgibbon ?

I think I have a few comments, please give me until tomorrow.

sorelfitzgibbon

LGTM. Just a few minor comments.

module/run_HS_metrics.nf

module/depth_to_bed.nf

sorelfitzgibbon · 2023-11-29T19:33:54Z

config/methods.config

This is probably part of the "clean up" @yashpatel6 suggested leaving for another PR: Much of the methods.config is out of date and could be replaced with those in pipeline-Nextflow-config as a submodule.

alkaZeltser · 2023-12-01T21:40:34Z

Generally looks good! There are some things that can be cleaned up but not needed in this PR. Can you move the outputs with the checksums to the dev directory?

Done!
/hot/software/pipeline/pipeline-targeted-coverage/Nextflow/development/unreleased/nzeltser-organize-for-metapipeline/all-outputs-with-checksums

alkaZeltser added 15 commits October 30, 2023 13:09

add bedops docker

bee3a4b

add process for merging off and on-target intervals

1edb483

add depth filtering process

5af09b8

handle various output conditions

d4584df

update template

f38a622

incorporate merged target file workflow

5227839

add flow diagram

8997af7

replace bedops with bash+bedtools workaround

c06584e

update image filename

68c3fd7

flow diagram updates

9853823

update readme

1ab4b16

update template config

8a7180a

add near_distance param to metrics calc

3426934

add bedtools-based merge bedfiles process

de544ea

add some documentation

7841278

alkaZeltser requested review from sorelfitzgibbon and yashpatel6 November 8, 2023 23:05

yashpatel6 reviewed Nov 10, 2023

View reviewed changes

PR comment fixes

6d3bd55

alkaZeltser added 3 commits November 27, 2023 16:55

add checksums

ee13fad

update configs

a4b1800

update changelog

3c1a527

alkaZeltser requested a review from yashpatel6 November 28, 2023 00:58

yashpatel6 approved these changes Nov 29, 2023

View reviewed changes

main.nf Outdated Show resolved Hide resolved

sorelfitzgibbon approved these changes Nov 29, 2023

View reviewed changes

directory path cleanup

e2a641f

This was referenced Dec 1, 2023

Convert diagram to PlantUML #24

Closed

put the processes under each if statement into their own workflows #25

Open

Update methods.config to use external module #26

Closed

alkaZeltser merged commit 1579289 into main Dec 1, 2023
1 check passed

alkaZeltser deleted the nzeltser-organize-for-metapipeline branch December 1, 2023 22:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

organize for metapipeline #23

organize for metapipeline #23

alkaZeltser commented Nov 8, 2023 •

edited

Loading

yashpatel6 left a comment

yashpatel6 Nov 10, 2023

yashpatel6 Nov 10, 2023

yashpatel6 commented Nov 10, 2023

alkaZeltser commented Nov 10, 2023

yashpatel6 commented Nov 17, 2023

yashpatel6 left a comment

sorelfitzgibbon commented Nov 29, 2023

sorelfitzgibbon left a comment

sorelfitzgibbon Nov 29, 2023

alkaZeltser commented Dec 1, 2023

organize for metapipeline #23

organize for metapipeline #23

Conversation

alkaZeltser commented Nov 8, 2023 • edited Loading

Description

Testing Results

Checklist

yashpatel6 left a comment

Choose a reason for hiding this comment

yashpatel6 Nov 10, 2023

Choose a reason for hiding this comment

yashpatel6 Nov 10, 2023

Choose a reason for hiding this comment

yashpatel6 commented Nov 10, 2023

alkaZeltser commented Nov 10, 2023

yashpatel6 commented Nov 17, 2023

yashpatel6 left a comment

Choose a reason for hiding this comment

sorelfitzgibbon commented Nov 29, 2023

sorelfitzgibbon left a comment

Choose a reason for hiding this comment

sorelfitzgibbon Nov 29, 2023

Choose a reason for hiding this comment

alkaZeltser commented Dec 1, 2023

alkaZeltser commented Nov 8, 2023 •

edited

Loading