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

Batch update: copy max_* params to resourcelimit directive #758

Merged
merged 17 commits into from
Oct 7, 2024

Conversation

jfy133
Copy link
Member

@jfy133 jfy133 commented Sep 23, 2024

This is in preparation for the new nf-core/tools 3.0.0 release, that deprecates the check_max() function + max_() params with the native Nextflow 'resourceLimit' directive.

I have done an initial pass using a bash script, but there are some remaining profiles that need to be addressed.

This will need to be dealt with manually.

Done Config Issue
x conf/alice.config Multiple process blocks
x conf/arcc.config Multiple process blocks
x conf/azurebatch.config Multiple process blocks
x conf/azurebatchdev.config Multiple process blocks
x conf/crg.config Multiple process blocks
x conf/czbiohub_aws.config Multiple process blocks
x conf/eva.config Multiple process blocks
x conf/hasta.config Multiple process blocks
x conf/hki.config Multiple process blocks
x conf/imperial.config Multiple process blocks
x conf/incliva.config Multiple process blocks
x conf/maestro.config Multiple process blocks
x conf/medair.config Multiple process blocks
x conf/pawsey_nimbus.config Multiple process blocks
x conf/rosalind_uge.config Multiple process blocks
x conf/sage.config Multiple process blocks
x conf/sanger.config Multiple process blocks
x conf/uge.config Multiple process blocks
x conf/uppmax.config Multiple process blocks
x conf/vsc_calcua.config Multiple process blocks
x conf/vsc_kul_uhasselt.config Multiple process blocks
x conf/vsc_ugent.config Multiple process blocks
x crukmi Uses check_max()
x rosalind_uge Unused check_max()
x sage Uses check_max()
x sahmri Uses check_max()
x uge Unused check_max()
x utd_ganymede Uses check_max()

To be adressed in follow up PR

Done Config Issue
pipeline/eager/eva Uses check_max()
pipeline/eager/maestro Uses check_max()
pipeline/eager/mag Uses check_max()
pipeline/rnafusion/hasta Uses check_max()
pipeline/taxprofiler/hasta Uses check_max()

Scripts and log:
batch-resourcelimit-scripts.zip

conf/crukmi.config Outdated Show resolved Hide resolved
Co-authored-by: Simon Pearce <[email protected]>
@edmundmiller
Copy link
Contributor

Go for it for my configs!

Copy link
Member

@mahesh-panchal mahesh-panchal left a comment

Choose a reason for hiding this comment

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

I think the uppmax profile should have the full resource limits map set. I have a feeling inheritance doesn't apply here.

conf/uppmax.config Show resolved Hide resolved
conf/uppmax.config Show resolved Hide resolved
conf/uppmax.config Outdated Show resolved Hide resolved
conf/uppmax.config Show resolved Hide resolved
@jfy133 jfy133 changed the title [WIP] Batch update: copy max_* params to resourcelimit directive Batch update: copy max_* params to resourcelimit directive Oct 4, 2024
@jfy133
Copy link
Member Author

jfy133 commented Oct 4, 2024

@HaidYi I've had to tweak your St. Jude profile slightly to make the tests pass (not sure why it didn't come up in your original PR), it bascially uses a Groovy call to resolve the TMPDIR path, because otherwise Nextflow thinks it's it's own variable.

I hope that's OK

Copy link
Member

@JoseEspinosa JoseEspinosa left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@mirpedrol mirpedrol merged commit 21a7b24 into nf-core:master Oct 7, 2024
132 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants