search

dask / dask-jobqueue

Deploy Dask on job schedulers like PBS, SLURM, and SGE
https://jobqueue.dask.org
BSD 3-Clause "New" or "Revised" License
235 stars 142 forks source link

Do not skip job_extra_directives with header_skip. Rename header_skp. #584

Closed guillaumeeb closed 2 years ago

guillaumeeb commented 2 years ago

Fixes #461.

I didn't thought I would make so many changes for this one...

But I thought I'd need to rename header_skip to be more in line with last changes. And then I also run into problems with SGECluster header that was not created as the other one. There is also HTCondorCluster wihch is a special case.

In the end, I prefer the code this way, but I would be happy to have some comments, maybe from @jolange, @riedel or @mivade who have been a bit active here lately.

Also keep in mind that at one point this code might completely change if we finally implement #338.

riedel commented 2 years ago

I actually never used this, however, I really find it a bit strange to use because you really need to know what the code will generate to then suppress it (The question is how to know if "#SBATCH -p" or "#SBATCH --partition=" so it implicitly fixes contract far below API level)

You also will get weird side effects eg. with header_skip=["-N"] if eg. the queue is named "Queue-New" (I guess it would be a valid queue name) or a jobname contains a minus something.

Then again what is the use case? Is there any variable I cannot simply set to none to supress the generation of default directives? I found something here:

https://github.com/dask/dask-blog/blob/58a93bceed4be9fbfb215e1be5200f0dcbce866a/_posts/2019-08-28-dask-on-summit.md?plain=1#L143

Which is the thing I do not understand: why would you first set the memory requested and then remove it from the request.

If I would want to support such hacking I would rather just support a generic post_process script that you can pass the whole submit file through if you really like to do weird stuff.

After making such a probably unsubstantial rant: your pull request actually makes things only better :)

guillaumeeb commented 2 years ago

Thanks for your valuable comment @riedel!

I agree this is a really advanced feature, but I've seen it used a few times, not only by Matthew Rocklin on Summit cluster. It's clearly a bit hacky, and hopefully, we'll make things better at some point if we implement #338.

Anyway, I'll stick happily with your final line:

After making such a probably unsubstantial rant: your pull request actually makes things only better :)