Reorganize the dask-jobqueue config file about dask-jobqueue HOT 20 CLOSED

We might also consider having separate yaml files sge.yaml, slurm.yaml, pbs.yaml

from dask-jobqueue.

That would also be a reasonable choice. Open to ideas here.

from dask-jobqueue.

It would be good to come to a decision here soonish. I think that this is the sort of issue might reasonably block the release of Dask proper. dask/dask#3592

from dask-jobqueue.

I'm currently in favor of separate files

from dask-jobqueue.

I'm fine with separate files. @lesteve and @guillaumeeb, any thoughts here?

from dask-jobqueue.

I don't have a strong opinion on this. If separated files, we should still keep some 'jobqueue' prefix, don't you think?
What will be the impact in the code? Each Cluster implementation will use all the keywords by default?

from dask-jobqueue.

from dask-jobqueue.

I just find it weird to have a .config/dask directory that looks like this:

distributed.yaml
dask.yaml
pbs.yaml
sge.yaml

I would prefer a more explicit naming (and more consistent with other packages):

distributed.yaml
dask.yaml
jobqueue-pbs.yaml
jobqueue-sge.yaml

from dask-jobqueue.

from dask-jobqueue.

Hrm, in trying to do this we now have to repeat all keyword arguments in all constructors. Are we ok with this? Also, what should we do for things like MOAB?

from dask-jobqueue.

I'm also now against the idea of splitting into multiple files, especially as more job schedulers come online.

from dask-jobqueue.

What if we implemented some basic inheritance?

from dask-jobqueue.

Sorry, let me be more explicit. Currently the core superclass constructor looks like this

class JobQueueCluster(Cluster):
    def __init__(self,
                 name=dask.config.get('jobqueue.name'),
                 threads=dask.config.get('jobqueue.threads'),
                 processes=dask.config.get('jobqueue.processes'),
                 memory=dask.config.get('jobqueue.memory'),
                 interface=dask.config.get('jobqueue.interface'),
                 death_timeout=dask.config.get('jobqueue.death-timeout'),
                 local_directory=dask.config.get('jobqueue.local-directory'),
                 extra=dask.config.get('jobqueue.extra'),
                 env_extra=dask.config.get('jobqueue.env-extra'),
                 **kwargs
                 ):
        """ """

If we now have separate config values for each resource manager then our current use of inheritance becomes less useful. Probably we'll get default values at runtime rather than import time, which is probably fine.

from dask-jobqueue.

So we would have something like the following:

class JobQueueCluster(Cluster):
    def __init__(self,
                 name=None,
                 threads=None
                 processes=None,
...
                 **kwargs
                 ):

        if name is None:
            name = dask.config.get('jobqueue.%s.name' % self._config_name)
        if ...

from dask-jobqueue.

Yep, I agree that we should not split the config into multiple files in the end.

What @mrocklin was saying earlier was also what I was affraid of, I don't really like the idea of repeating all the keywords into each subclass __init__. The last proposed solution seems much better though, even if it add some complexity, it does take advantage of inheritance.

Rather than _config_name, we could use _scheduler_name, which might be usefull in the future for other purpose (or not ...).

We should also clearly define what goes into the pbs/slurm/sge categories, all of the values? Or should we keep some in the jobqueue level?

I personnaly have no use of this functionnality, and I believe (but I may be wrong) that this is true for most users. But the current proposed solution is not too intrusive into the scheduler subclass code, and replacing

jobqueue:
  name: dask-worker
  threads: 2
  processes: 4
  memory: 8GB
  interface: null
  death-timeout: 60
  local-directory: null
  extra: ""
  env-extra: []

  queue: null
  project: null
  walltime: '00:30:00'

  pbs:
    resource-spec: null
    job-extra: []

By

jobqueue:
  # dask options
  name: dask-worker
  death-timeout: 60

  pbs:
    threads: 2
    processes: 4
    memory: 8GB
    interface: null
    local-directory: null
    extra: ""
    env-extra: []
    queue: null
    project: null
    walltime: '00:30:00'
    resource-spec: null
    job-extra: []

in my config file is fine for me.

from dask-jobqueue.

I personnaly have no use of this functionnality, and I believe (but I may be wrong) that this is true for most users. But the current proposed solution is not too intrusive into the scheduler subclass code, and replacing

Right, so we could also just decide not to make this change. @jhamman is this a concrete need, or is this more hypothetical? If the latter then do you have objections to waiting until a user raises this as an issue?

from dask-jobqueue.

is this a concrete need

Yes, Cheyenne uses two schedulers (slurm for the dav cluster, pbs for cheyenne proper).

from dask-jobqueue.

from dask-jobqueue.

Not that I am aware of. Typically, that would be handled by using separate queues for the different clusters.

from dask-jobqueue.

OK, I've given this a shot in #70

from dask-jobqueue.