allow slurm to be used as scheduler

[adamcallison]

the other commit just moves the part that checks if the scheduler is valid to the top of gen_qsub_script

[adamcallison]

One issue with this is that it could be confusing that a slurm submission is invoked by grow_qsub (and similar for gen_qsub_script) as slurm uses sbatch instead of qsub, but I thought an entirely new function was overkill as most of the logic is the same. Considering renaming these as grow_cluster and gen_cluster_script or similar, and just providing grow_qsub and gen_qsub_script as thin wrappers so nothing breaks. Thoughts?

[adamcallison]

Also, this has been (as usual with this qsub stuff) difficult to test. I have used this to run stuff on a slurm cluster but I've not tried all the options.

[jcmgray]

Great! Yes they have slurm here and I was planning to add this soon but glad to be beaten to it ;)

I think probably whats makes sense is yes: grow_cluster and gen_cluster_script, with the scheduler argument no longer optional. Then

crop.grow_sge
crop.grow_pbs
crop.glow_slurm

for convenience & ideally a deprecation warning for using the now slightly ambiguous qsub_grow. What do you think?

I can add these things if you don't fancy doing them in this PR!

[pep8speaks]

Hello @adamcallison! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2020-10-22 11:41:46 UTC

[adamcallison]

Forgot to do the pep8 check. Will do that now.

I implemented what we discussed. I added DeprecationWarnings where appropriate but it seems that they are ignored by default in python. Do you know what the correct approach is here?

[adamcallison]

Also, I think it's worth you taking a look at how I've done the extra_resources arg for slurm. (Either before accepting the PR or once you start using it). It works fine but could be handled differently - it's not clear to me what the right way is.

[AdrianSosic]

Hi @adamcallison and @jcmgray, I am currently also considering to use xyzpy in combination with slurm and therefore just wanted to ask if you are still actively working on this feature (since there haven't been any further comments in the last few months). Kind regards!

[adamcallison]

@AdrianSosic and @jcmgray I had some difficulties getting the partialmethod objects to document themselves, since functools.update_wrapper doesn't seem to work on them, so I never finished this properly. I've just added a commit that removes the module level grow_sge etc functions and just adds them to Crop as partialmethod objects and doesn't attempt to add a documentation wrapper because I can't figure out how to do it. The commit should work in terms of functionality (although would benefit from more 'in the wild' testing). @jcmgray what do you think?

[jcmgray]

Yeah all looks good to me! Sorry its taken me a while to get back to this, but I am looking forward to trying it. Thanks for the contribution @adamcallison. @AdrianSosic do let us know if you try it and have any problems - this kind of functionality is hard to test.

[AdrianSosic]

Great, thanks for adding the commit! Appreciate! I will try it out and definitely give you feedback if I encounter any problems.