8000 [air] Add _max_cpu_fraction_per_node to ScalingConfig and documentation by ericl · Pull Request #26634 · ray-project/ray · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[air] Add _max_cpu_fraction_per_node to ScalingConfig and documentation #26634

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 12 commits into from
Jul 17, 2022

Conversation

ericl
Copy link
Contributor
@ericl ericl commented Jul 16, 2022

Why are these changes needed?

As a followup for #26397, add this to the docs and API as an experimental feature.

ericl added 4 commits July 16, 2022 16:43
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
tune.run(trainer.as_trainable(), num_samples=4)


# TODO(ekl/sang) this currently fails.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rkooo567 , it seems this fails since all CPUs end up excluded. Can we ensure at least 1 CPU is available on either side no matter how aggressive the fraction is?

Btw, I think we should disallow 1.0 and 0.0 as values (raise ValueError).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #26635

Comment on lines 162 to 163
``_max_cpu_fraction_per_node`` is experimental and not recommended for use with
autoscaling clusters.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be more explicit and say that the reason is that doing so may cause deadlock?

ericl added 3 commits July 16, 2022 17:41
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
ericl added 4 commits July 16, 2022 17:43
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
8000
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
@ericl ericl added the do-not-merge Do not merge this PR! label Jul 17, 2022
@ericl ericl merged commit 400330e into ray-project:master Jul 17, 2022
jianoaix pushed a commit to jianoaix/ray that referenced this pull request Jul 18, 2022
…on (ray-project#26634)

Signed-off-by: Ubuntu <ubuntu@ip-172-31-32-136.us-west-2.compute.internal>
xwjiang2010 pushed a commit to xwjiang2010/ray that referenced this pull request Jul 19, 2022
…on (ray-project#26634)

Signed-off-by: Xiaowei Jiang <xwjiang2010@gmail.com>
Stefan-1313 pushed a commit to Stefan-1313/ray_mod that referenced this pull request Aug 18, 2022
…on (ray-project#26634)

Signed-off-by: Stefan van der Kleij <s.vanderkleij@viroteq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge Do not merge this PR!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0