Skip to content

Update .scale() and .adapt() docstrings #346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 8, 2019

Conversation

andersy005
Copy link
Member

@andersy005 andersy005 commented Oct 1, 2019

Addresses #344 (comment)

I am not sure what's the right way to include the information from distributed's SpecCluster without copying and pasting it into the new docstrings:

  • adapt(): Updated
In [4]: cluster.adapt?                                                                                          
Signature:
cluster.adapt(
    *args,
    minimum_jobs: int = None,
    maximum_jobs: int = None,
    **kwargs,
)
Docstring:
Scale Dask cluster automatically based on scheduler activity.

Parameters
----------

minimum_jobs : int
   Minimum number of jobs
maximum_jobs : int
   Maximum number of jobs
**kwargs :
   Extra parameters to pass to dask.distributed.Adaptive
  • adapt(): Old version
In [4]: cluster.adapt?                                                                                          
Signature:
cluster.adapt(
    minimum_cores=None,
    maximum_cores=None,
    minimum_memory=None,
    maximum_memory=None,
    **kwargs,
)
Docstring:
Turn on adaptivity
For keyword arguments see dask.distributed.Adaptive
Instead of minimum and maximum parameters which apply to the number of
worker, If Cluster object implements jobqueue_worker_spec attribute, one can
use the following parameters:
Parameters
----------
minimum_cores: int
    Minimum number of cores for the cluster
maximum_cores: int
    Maximum number of cores for the cluster
minimum_memory: str
    Minimum amount of memory for the cluster
maximum_memory: str
    Maximum amount of memory for the cluster
Examples
--------
>>> cluster.adapt(minimum=0, maximum=10, interval='500ms')
>>> cluster.adapt(minimum_cores=24, maximum_cores=96)
>>> cluster.adapt(minimum_memory='60 GB', maximum_memory= '1 TB')
  • scale
                                                                      
In [3]: cluster.scale?                                                                                          
Signature: cluster.scale(n=None, jobs=0, memory=None, cores=None)
Docstring:
Scale cluster to specified configurations.

Parameters
----------
n : int
   Target number of workers
jobs : int
   Target number of jobs
memory : str
   Target amount of memory
cores : int
   Target number of cores

Copy link
Member

@jhamman jhamman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @andersy005 for the doc improvements! Just one suggestion to make the adaptive scaling method a bit clearer.


Parameters
----------

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also include the minimum/maximum arguments here. I suspect most users will interact with these regularly and I think chasing the arguments through to dask.distributed.Adaptive will end up being annoying.

@andersy005
Copy link
Member Author

@jhamman,

When you get time, this is ready for another look.

Comment on lines 519 to 522
minimum_cores : int
Minimum number of cores for the cluster
maximum_cores : int
Maximum number of cores for the cluster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want the _cores in these names. These will be passed through to https://distributed.dask.org/en/latest/api.html#adaptive which just uses minimum/maximum.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Fixed it ( I think :))

@jhamman jhamman merged commit 6f63d4a into dask:master Oct 8, 2019
@andersy005 andersy005 deleted the update-scale-adapt-docs branch October 8, 2019 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants