Skip to content

[WIP] Placement Group API for Python and its mock implementation#8556

Closed
yuyiming wants to merge 1 commit intoray-project:masterfrom
antgroup:placement_group_python
Closed

[WIP] Placement Group API for Python and its mock implementation#8556
yuyiming wants to merge 1 commit intoray-project:masterfrom
antgroup:placement_group_python

Conversation

@yuyiming
Copy link
Copy Markdown
Contributor

Documents for Placement Group Design and API:

Work Items

  • Placement Group creation API;
    • Its mock implementation;
  • Actor Creation API with specified BundleId (Placement Group Id);
    • Its mock implementation;

Why are these changes needed?

Python version of Placement API. The previous Java API is in PR #7712.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/latest/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failure rates at https://ray-travis-tracker.herokuapp.com/.
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested (please justify below)

@AmplabJenkins
Copy link
Copy Markdown

Can one of the admins verify this patch?

@AmplabJenkins
Copy link
Copy Markdown

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/26227/
Test PASSed.

@ericl ericl self-assigned this May 22, 2020
@rkooo567 rkooo567 self-requested a review May 22, 2020 18:33
def test_placement_group(ray_start_10_cpus):
group = ray.experimental.placement_group("test_group")\
.set_strategy(PlacementStrategy.SPREAD)\
.add_bundle({"CPU": 2.0}).add_bundle({"CPU": 1.0}).create()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For python, we probably want these to be kwargs instead of using the builder pattern.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ray.experimental.placement_group(name="test_group", strategy="spread", bundles=[{"CPU": 2}] * 10)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

@enum.unique
class PlacementStrategy(enum.Enum):
PACK = 0
SPREAD = 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For Python we should just use strategy="pack" or strategy="spread" for simplicity, and raise ValueError if the argument is not one of these.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LGTM. String is much simpler than enumeration.

@rkooo567
Copy link
Copy Markdown
Contributor

rkooo567 commented Jun 3, 2020

Hi, guys. What's the status for this PR?

@ericl
Copy link
Copy Markdown
Contributor

ericl commented Jun 18, 2020

Going to close these now that we have the real implementation PR out.

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.

4 participants