nextflow
nextflow copied to clipboard
Modify slurm executor to support cluster [closes #2473]
Hi @pditommaso ,
I had a stab at adding the cluster directive. This is my first contact with your project and with Groovy so I hope I did not make to many mistakes.
It is important to note that this is cluster
not clusters
in the doc as allowing multiple would overly complicate scancel and squeue
Thanks for opening this PR. It looks a good start. Not sure to understand the comment below, though
It is important to note that this is cluster not clusters in the doc as allowing multiple would overly complicate scancel and squeue
Thanks for opening this PR. It looks a good start. Not sure to understand the comment below, though
It is important to note that this is cluster not clusters in the doc as allowing multiple would overly complicate scancel and squeue
So if we look at the documentation for sbatch for instance https://slurm.schedmd.com/sbatch.html we can see that -M
(long format --clusters
) accepts multiple clusters (e.g. zeus,magnus
) and the job gets submitted to the first cluster that has available slots. By not covering that case but just allowing a single cluster
, we probably cover most cases everyone cares about without the added complexity of querying squeue to find out where the job was actually submitted.
My comment is a reminder that before merging there should be a NOTICE
box in the documentation that informs users about this distinction
I have to add that this PR is not yet ready as I seem to be having issues when running this. I will comment back when ready and will keep updating the commit.
I have to add that this PR is not yet ready as I seem to be having issues when running this. I will comment back when ready and will keep updating the commit.
Ok @pditommaso this PR has worked for me as it is at the moment. Please have a look
Update: fixed oversight that I found in testing