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

Update config.yaml #142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

wlason
Copy link

@wlason wlason commented Oct 22, 2024

This is how I would suggest to update the slurm config.yaml

It adds a sensible default number of threads for HPC and fixes the small typo in specifying the cluster command.

It also creates logs which are prefixed with rule name (similarly to what snakemake would do if you run it with --slurm instead of specifying --profile slurm - if you do that, it would natively save to slurm_logs/rule_{rule}/%j.log in .snakemake directory), which are easier to read in case of errors - it allowed me to work out which rule needed more memory, for example.

@quentinblampey
Copy link
Collaborator

quentinblampey commented Oct 24, 2024

Thanks @wlason, this looks good to me!
I'm currently out-of-office, I'll check that it also work on our clusters as an additional check, and then merge it 👍

- gpu=0
- threads=8
- time="30:00:00"
Copy link
Collaborator

@quentinblampey quentinblampey Oct 24, 2024

Choose a reason for hiding this comment

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

Minor comment: this might be something that doesn't fit well for all users (mostly the "time" line). I think this is something that the user should add if needed. Or maybe this can be added as a comment, and any user can remove the comment if needed.
WDYT?

@quentinblampey
Copy link
Collaborator

quentinblampey commented Nov 14, 2024

Hello @wlason, sorry for the delay, I had unexpected deadlines.
Could you please update your PR as asked in the comment above? (basically, keeping the changes until line 11 included).
Then I'll merge it :)

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.

2 participants