Switch to using generalized resources #90

nwiltsie · 2024-11-15T19:18:12Z

Description

This updates the pipeline to use generalized resources, as detailed in this comment.

I've updated the pipeline-Nextflow-config submodule to uclahs-cds/pipeline-Nextflow-config@c612346 (currently the head of uclahs-cds/pipeline-Nextflow-config#73). That PR should be merged and the submodule updated again before this PR is merged.

Testing Results

NFTest
- log: /hot/software/pipeline/pipeline-recalibrate-BAM/Nextflow/development/unreleased/nwiltsie-generalize-resources/log-nftest-20241115T182709Z.log
- cases: default set

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 using NFTest, or I have justified why I did not need to run NFTest above.

github-actions · 2024-11-15T19:19:32Z

Bleep bloop, I am a robot.

Alas, some of the Nextflow configuration tests failed!

test/configtest-F16.json

@ ["params","proc_resource_params"]
- {"deduplicate_records_SAMtools":{"cpus":"2","memory":"27.9 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"extract_GenomeIntervals":{"cpus":"1","memory":"1 GB"},"remove_intermediate_files":{"cpus":"1","memory":"1 GB"},"remove_merged_BAM":{"cpus":"1","memory":"1 GB"},"remove_unmerged_BAMs":{"cpus":"1","memory":"1 GB"},"run_ApplyBQSR_GATK":{"cpus":"1","memory":"2 GB","retry_strategy":{"memory":{"operand":"4","strategy":"exponential"}}},"run_BaseRecalibrator_GATK":{"cpus":"1","memory":"27.9 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_CalculateContamination_GATK":{"cpus":"1","memory":"14 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_DepthOfCoverage_GATK":{"cpus":"1","memory":"14 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_GetPileupSummaries_GATK":{"cpus":"1","memory":"14 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_IndelRealigner_GATK":{"cpus":"2","memory":"4 GB","retry_strategy":{"memory":{"operand":"4","strategy":"exponential"}}},"run_MergeSamFiles_Picard":{"cpus":"2","memory":"27.9 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_RealignerTargetCreator_GATK":{"cpus":"2","memory":"4 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_SplitIntervals_GATK":{"cpus":"1","memory":"1 GB"},"run_index_SAMtools":{"cpus":"1","memory":"2 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_validate_PipeVal":{"cpus":"1","memory":"1 GB"}}
@ ["params","base_allocations"]
+ {"deduplicate_records_SAMtools":{"cpus":"2","memory":"8 GB"},"extract_GenomeIntervals":{"cpus":"1","memory":"1 GB"},"remove_intermediate_files":{"cpus":"1","memory":"1 GB"},"remove_merged_BAM":{"cpus":"1","memory":"1 GB"},"remove_unmerged_BAMs":{"cpus":"1","memory":"1 GB"},"run_ApplyBQSR_GATK":{"cpus":"1","memory":"2 GB"},"run_BaseRecalibrator_GATK":{"cpus":"1","memory":"30 GB"},"run_CalculateContamination_GATK":{"cpus":"1","memory":"5 GB"},"run_DepthOfCoverage_GATK":{"cpus":"1","memory":"5 GB"},"run_GetPileupSummaries_GATK":{"cpus":"1","memory":"5 GB"},"run_IndelRealigner_GATK":{"cpus":"2","memory":"4 GB"},"run_MergeSamFiles_Picard":{"cpus":"2","memory":"8 GB"},"run_RealignerTargetCreator_GATK":{"cpus":"2","memory":"4 GB"},"run_SplitIntervals_GATK":{"cpus":"1","memory":"1 GB"},"run_index_SAMtools":{"cpus":"1","memory":"2 GB"},"run_validate_PipeVal":{"cpus":"1","memory":"1 GB"}}
@ ["params","max_time"]
+ "24d 20h 31m 24s"
@ ["params","min_time"]
+ "1s"
@ ["params","retry_information"]
+ {"deduplicate_records_SAMtools":{"memory":{"operand":"2","strategy":"exponential"}},"run_ApplyBQSR_GATK":{"memory":{"operand":"4","strategy":"exponential"}},"run_BaseRecalibrator_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_CalculateContamination_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_DepthOfCoverage_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_GetPileupSummaries_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_IndelRealigner_GATK":{"memory":{"operand":"4","strategy":"exponential"}},"run_MergeSamFiles_Picard":{"memory":{"operand":"2","strategy":"exponential"}},"run_RealignerTargetCreator_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_index_SAMtools":{"memory":{"operand":"2","strategy":"exponential"}}}
@ ["process","cpus"]
- {"1":"1","2":"2","3":"3","closure":"closure()"}
+ "16"
@ ["process","errorStrategy"]
- {"1":"terminate","2":"terminate","3":"terminate","closure":"terminate"}
@ ["process","maxRetries"]
- "1"
@ ["process","withLabel:process_high"]
- {"cpus":{"1":"12","2":"12","3":"12","closure":"retry_updater(12, add, 0, $task.attempt, cpus)"},"memory":{"1":"31 GB","2":"31 GB","3":"31 GB","closure":"retry_updater(84 GB, exponential, 2, $task.attempt, memory)"}}
@ ["process","withLabel:process_low"]
- {"cpus":{"1":"2","2":"2","3":"2","closure":"retry_updater(2, add, 0, $task.attempt, cpus)"},"memory":{"1":"3 GB","2":"6 GB","3":"12 GB","closure":"retry_updater(3 GB, exponential, 2, $task.attempt, memory)"}}
@ ["process","withLabel:process_medium"]
- {"cpus":{"1":"6","2":"6","3":"6","closure":"retry_updater(6, add, 0, $task.attempt, cpus)"},"memory":{"1":"31 GB","2":"31 GB","3":"31 GB","closure":"retry_updater(42 GB, exponential, 2, $task.attempt, memory)"}}
@ ["process","withName:deduplicate_records_SAMtools","memory"]
- {"1":"27.9 GB","2":"31 GB","3":"31 GB","closure":"retry_updater(27.9 GB, exponential, 2, $task.attempt, memory)"}
+ "27.9 GB"
@ ["process","withName:run_BaseRecalibrator_GATK","memory"]
- {"1":"27.9 GB","2":"31 GB","3":"31 GB","closure":"retry_updater(27.9 GB, exponential, 2, $task.attempt, memory)"}
+ "27.9 GB"
@ ["process","withName:run_CalculateContamination_GATK","memory"]
- {"1":"14 GB","2":"27.9 GB","3":"31 GB","closure":"retry_updater(14 GB, exponential, 2, $task.attempt, memory)"}
+ "14 GB"
@ ["process","withName:run_DepthOfCoverage_GATK","memory"]
- {"1":"14 GB","2":"27.9 GB","3":"31 GB","closure":"retry_updater(14 GB, exponential, 2, $task.attempt, memory)"}
+ "14 GB"
@ ["process","withName:run_GetPileupSummaries_GATK","memory"]
- {"1":"14 GB","2":"27.9 GB","3":"31 GB","closure":"retry_updater(14 GB, exponential, 2, $task.attempt, memory)"}
+ "14 GB"
@ ["process","withName:run_MergeSamFiles_Picard","memory"]
- {"1":"27.9 GB","2":"31 GB","3":"31 GB","closure":"retry_updater(27.9 GB, exponential, 2, $task.attempt, memory)"}
+ "27.9 GB"
@ ["resource_handler"]
+ {"handle_resources":"closure()","set_resource_limit_params":"closure()","set_retry":"closure()","setup_retry":"closure()"}

test/configtest-F32.json

@ ["params","proc_resource_params"]
- {"deduplicate_records_SAMtools":{"cpus":"2","memory":"57.6 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"extract_GenomeIntervals":{"cpus":"1","memory":"1 GB"},"remove_intermediate_files":{"cpus":"1","memory":"1 GB"},"remove_merged_BAM":{"cpus":"1","memory":"1 GB"},"remove_unmerged_BAMs":{"cpus":"1","memory":"1 GB"},"run_ApplyBQSR_GATK":{"cpus":"1","memory":"2 GB","retry_strategy":{"memory":{"operand":"4","strategy":"exponential"}}},"run_BaseRecalibrator_GATK":{"cpus":"1","memory":"57.6 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_CalculateContamination_GATK":{"cpus":"1","memory":"28.8 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_DepthOfCoverage_GATK":{"cpus":"1","memory":"28.8 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_GetPileupSummaries_GATK":{"cpus":"1","memory":"28.8 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_IndelRealigner_GATK":{"cpus":"4","memory":"6 GB","retry_strategy":{"memory":{"operand":"4","strategy":"exponential"}}},"run_MergeSamFiles_Picard":{"cpus":"2","memory":"57.6 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_RealignerTargetCreator_GATK":{"cpus":"4","memory":"6 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_SplitIntervals_GATK":{"cpus":"1","memory":"1 GB"},"run_index_SAMtools":{"cpus":"1","memory":"2 GB","retry_strategy":{"memory":{"operand":"2","strategy":"exponential"}}},"run_validate_PipeVal":{"cpus":"1","memory":"1 GB"}}
@ ["params","base_allocations"]
+ {"deduplicate_records_SAMtools":{"cpus":"2","memory":"8 GB"},"extract_GenomeIntervals":{"cpus":"1","memory":"1 GB"},"remove_intermediate_files":{"cpus":"1","memory":"1 GB"},"remove_merged_BAM":{"cpus":"1","memory":"1 GB"},"remove_unmerged_BAMs":{"cpus":"1","memory":"1 GB"},"run_ApplyBQSR_GATK":{"cpus":"1","memory":"2 GB"},"run_BaseRecalibrator_GATK":{"cpus":"1","memory":"60 GB"},"run_CalculateContamination_GATK":{"cpus":"1","memory":"5 GB"},"run_DepthOfCoverage_GATK":{"cpus":"1","memory":"10 GB"},"run_GetPileupSummaries_GATK":{"cpus":"1","memory":"5 GB"},"run_IndelRealigner_GATK":{"cpus":"4","memory":"6 GB"},"run_MergeSamFiles_Picard":{"cpus":"2","memory":"8 GB"},"run_RealignerTargetCreator_GATK":{"cpus":"4","memory":"6 GB"},"run_SplitIntervals_GATK":{"cpus":"1","memory":"1 GB"},"run_index_SAMtools":{"cpus":"1","memory":"2 GB"},"run_validate_PipeVal":{"cpus":"1","memory":"1 GB"}}
@ ["params","max_time"]
+ "24d 20h 31m 24s"
@ ["params","min_time"]
+ "1s"
@ ["params","retry_information"]
+ {"deduplicate_records_SAMtools":{"memory":{"operand":"2","strategy":"exponential"}},"run_ApplyBQSR_GATK":{"memory":{"operand":"4","strategy":"exponential"}},"run_BaseRecalibrator_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_CalculateContamination_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_DepthOfCoverage_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_GetPileupSummaries_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_IndelRealigner_GATK":{"memory":{"operand":"4","strategy":"exponential"}},"run_MergeSamFiles_Picard":{"memory":{"operand":"2","strategy":"exponential"}},"run_RealignerTargetCreator_GATK":{"memory":{"operand":"2","strategy":"exponential"}},"run_index_SAMtools":{"memory":{"operand":"2","strategy":"exponential"}}}
@ ["process","cpus"]
- {"1":"1","2":"2","3":"3","closure":"closure()"}
+ "32"
@ ["process","errorStrategy"]
- {"1":"terminate","2":"terminate","3":"terminate","closure":"terminate"}
@ ["process","maxRetries"]
- "1"
@ ["process","withLabel:process_high"]
- {"cpus":{"1":"12","2":"12","3":"12","closure":"retry_updater(12, add, 0, $task.attempt, cpus)"},"memory":{"1":"64 GB","2":"64 GB","3":"64 GB","closure":"retry_updater(84 GB, exponential, 2, $task.attempt, memory)"}}
@ ["process","withLabel:process_low"]
- {"cpus":{"1":"2","2":"2","3":"2","closure":"retry_updater(2, add, 0, $task.attempt, cpus)"},"memory":{"1":"3 GB","2":"6 GB","3":"12 GB","closure":"retry_updater(3 GB, exponential, 2, $task.attempt, memory)"}}
@ ["process","withLabel:process_medium"]
- {"cpus":{"1":"6","2":"6","3":"6","closure":"retry_updater(6, add, 0, $task.attempt, cpus)"},"memory":{"1":"42 GB","2":"64 GB","3":"64 GB","closure":"retry_updater(42 GB, exponential, 2, $task.attempt, memory)"}}
@ ["process","withName:deduplicate_records_SAMtools","memory"]
- {"1":"57.6 GB","2":"64 GB","3":"64 GB","closure":"retry_updater(57.6 GB, exponential, 2, $task.attempt, memory)"}
+ "57.6 GB"
@ ["process","withName:run_BaseRecalibrator_GATK","memory"]
- {"1":"57.6 GB","2":"64 GB","3":"64 GB","closure":"retry_updater(57.6 GB, exponential, 2, $task.attempt, memory)"}
+ "57.6 GB"
@ ["process","withName:run_CalculateContamination_GATK","memory"]
- {"1":"28.8 GB","2":"57.6 GB","3":"64 GB","closure":"retry_updater(28.8 GB, exponential, 2, $task.attempt, memory)"}
+ "28.8 GB"
@ ["process","withName:run_DepthOfCoverage_GATK","memory"]
- {"1":"28.8 GB","2":"57.6 GB","3":"64 GB","closure":"retry_updater(28.8 GB, exponential, 2, $task.attempt, memory)"}
+ "28.8 GB"
@ ["process","withName:run_GetPileupSummaries_GATK","memory"]
- {"1":"28.8 GB","2":"57.6 GB","3":"64 GB","closure":"retry_updater(28.8 GB, exponential, 2, $task.attempt, memory)"}
+ "28.8 GB"
@ ["process","withName:run_MergeSamFiles_Picard","memory"]
- {"1":"57.6 GB","2":"64 GB","3":"64 GB","closure":"retry_updater(57.6 GB, exponential, 2, $task.attempt, memory)"}
+ "57.6 GB"
@ ["resource_handler"]
+ {"handle_resources":"closure()","set_resource_limit_params":"closure()","set_retry":"closure()","setup_retry":"closure()"}

If the above changes are surprising, stop and determine what happened.

If the above changes are expected, there are two ways to fix this:

Automatically: Post a comment starting with "/fix-tests" (without the quotes) and I will update the tests for you (you must review my work afterwards).
Manually: Follow these steps on Confluence.

nwiltsie · 2024-11-15T19:21:39Z

These test changes should be carefully reviewed, as the logic generating them is at the core of this PR.

wiz-inc-8da00b022c · 2024-12-02T18:12:12Z

Wiz Scan Summary

Scanner	Findings
Vulnerabilities
Sensitive Data	1
Secrets
IaC Misconfigurations

Total	1

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

yashpatel6 · 2024-12-02T18:28:08Z

@nwiltsie This should now be an identical configuration in terms of allocations and setup; the differences in the JSONs are largely due to the base allocations now being split from the retry strategy per process if you want to confirm

nwiltsie · 2024-12-02T18:33:45Z

@nwiltsie This should now be an identical configuration in terms of allocations and setup; the differences in the JSONs are largely due to the base allocations now being split from the retry strategy per process if you want to confirm

Confirmed, the differences all seem structural and not quantitative. Thanks for fixing those up!

nwiltsie · 2024-12-18T17:43:31Z

uclahs-cds/pipeline-Nextflow-config#73 has been merged, so I've updated the submodule to point to main. This is ready for review!

yashpatel6 · 2024-12-18T20:28:43Z

I'll go over this today or tomorrow!

yashpatel6

Looks good! I tweaked some of the default allocations in 9125ad6

Tested with log: /hot/software/pipeline/pipeline-recalibrate-BAM/Nextflow/development/1.0.2/nwiltsie-generalize-resources/log-nftest-20241220T014616Z.log

If the tweaks look good, I'm good to merge this

nwiltsie added 5 commits November 15, 2024 10:09

Add resources.json

de721d4

Delete node-specific configs

096d7e6

Replace old calls with handle_resources()

87cca2f

Update submodule, update import

41b1da0

Update CHANGELOG

3276312

nwiltsie requested a review from yashpatel6 November 15, 2024 19:18

yashpatel6 self-assigned this Nov 15, 2024

yashpatel6 added 4 commits November 27, 2024 12:48

Update allocation function for custom allocations

de53e00

Update config submodule

e6b95c4

Update config submodule

19aeeba

Update configtests

e475464

Update submodule

c5b6c6f

nwiltsie marked this pull request as ready for review December 18, 2024 17:42

nwiltsie requested review from tyamaguchi-ucla and Faizal-Eeman as code owners December 18, 2024 17:42

nwiltsie requested a review from a team December 18, 2024 17:42

Adjust default resources with min and maxes

9125ad6

yashpatel6 approved these changes Dec 20, 2024

View reviewed changes

nwiltsie merged commit 9cf9c81 into main Dec 20, 2024
7 checks passed

nwiltsie deleted the nwiltsie-generalize-resources branch December 20, 2024 17:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to using generalized resources #90

Switch to using generalized resources #90

nwiltsie commented Nov 15, 2024

github-actions bot commented Nov 15, 2024

nwiltsie commented Nov 15, 2024

wiz-inc-8da00b022c bot commented Dec 2, 2024 •

edited

Loading

yashpatel6 commented Dec 2, 2024

nwiltsie commented Dec 2, 2024

nwiltsie commented Dec 18, 2024

yashpatel6 commented Dec 18, 2024

yashpatel6 left a comment

Switch to using generalized resources #90

Switch to using generalized resources #90

Conversation

nwiltsie commented Nov 15, 2024

Description

Testing Results

Checklist

github-actions bot commented Nov 15, 2024

test/configtest-F16.json

test/configtest-F32.json

nwiltsie commented Nov 15, 2024

wiz-inc-8da00b022c bot commented Dec 2, 2024 • edited Loading

Wiz Scan Summary

yashpatel6 commented Dec 2, 2024

nwiltsie commented Dec 2, 2024

nwiltsie commented Dec 18, 2024

yashpatel6 commented Dec 18, 2024

yashpatel6 left a comment

Choose a reason for hiding this comment

wiz-inc-8da00b022c bot commented Dec 2, 2024 •

edited

Loading