search

CoBrALab / qbatch

The Unlicense
27 stars 13 forks source link

Slurm cleanup #150

Closed gdevenyi closed 6 years ago

gdevenyi commented 6 years ago

With access to Niagara and a careful reading of sbatch documentation, the slurm implementation does not conform to how the SGE and PBS configurations work.

Fixing this up.

gdevenyi commented 6 years ago

Paging @dcoynel this will likely have some breaking changes based on your original implementation.

Please comment.

gdevenyi commented 6 years ago

@bcdarwin Have you used slurm yet, any comments on these?

bcdarwin commented 6 years ago

We use it on our local cluster and on Graham -- I'll try it out tomorrow.

gdevenyi commented 6 years ago

Thanks @bcdarwin, I appreciate the help!

Obviously next step is the fixup of the memory handling.

bcdarwin commented 6 years ago

Can you also merge afa1a5052b28 ?

gdevenyi commented 6 years ago

Ooops, my bad, I didn't realize I never merged that into master. Now squashed into this PR.

gdevenyi commented 6 years ago

Any other issues/comments @bcdarwin ?

bcdarwin commented 6 years ago

I thought I had mentioned it but it doesn't seem to be the case ... the --nodes=$n argument doesn't work on Slurm, but --options='--nodes=$n' does work. Unfortunately I don't have time today to figure out why, but it's also not an urgent fix to me.

gdevenyi commented 6 years ago

Hrm,

Can you clarify "doesn't work"?

A quick check with qbatch -n, it seems to generate the proper job wrap for slurm.

bcdarwin commented 6 years ago

Right ... the reason I didn't post this comment was because I previously realized I was on the wrong branch at the time. Everything seems fine to me ...

gdevenyi commented 6 years ago

Thanks for the feedback, will check for other small bugs to fix, then will probably stamp a new official release.