Skip to content
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

[core] PlacementGroupSchedulingStrategy does not support placement_group=None #27328

Closed
krfricke opened this issue Aug 1, 2022 · 2 comments · Fixed by #27370
Closed

[core] PlacementGroupSchedulingStrategy does not support placement_group=None #27328

krfricke opened this issue Aug 1, 2022 · 2 comments · Fixed by #27370
Assignees
Labels
bug Something that is supposed to be working; but isn't core Issues that should be addressed in Ray Core P0 Issues that should be fixed in short order release-blocker P0 Issue that blocks the release

Comments

@krfricke
Copy link
Contributor

krfricke commented Aug 1, 2022

What happened + What you expected to happen

The docstring says

    Attributes:
        placement_group: the placement group this actor belongs to,
            or None if it doesn't belong to any group.

but the implementation says:

        if placement_group is None:
            raise ValueError(
                "placement_group needs to be an instance of PlacementGroup"
            )

We should either adjust the docstring or allow None again.

It seems like passing scheduling_strategy="DEFAULT" achieves the same job, though this is not well documented.

Versions / Dependencies

Master, 2.0.0rc0

Reproduction script

from ray.util.scheduling_strategies import PlacementGroupSchedulingStrategy

PlacementGroupSchedulingStrategy(placement_group=None)

Issue Severity

Low: It annoys or frustrates me.

@krfricke krfricke added bug Something that is supposed to be working; but isn't triage Needs triage (eg: priority, bug/not-bug, and owning component) labels Aug 1, 2022
@rkooo567
Copy link
Contributor

rkooo567 commented Aug 1, 2022

cc @jjyao this looks like a regression (or a bug when introducing this API). Can you confirm that?

@jjyao
Copy link
Collaborator

jjyao commented Aug 1, 2022

Thanks @krfricke for catching this!

@rkooo567 I thought it's working and it's already mentioned in the ray pg doc. But apparently this is not the case and code snippet in the doc is not tested by CI so we didn't catch this earlier. I'll fix this.

@jjyao jjyao added release-blocker P0 Issue that blocks the release P0 Issues that should be fixed in short order core Issues that should be addressed in Ray Core and removed triage Needs triage (eg: priority, bug/not-bug, and owning component) labels Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is supposed to be working; but isn't core Issues that should be addressed in Ray Core P0 Issues that should be fixed in short order release-blocker P0 Issue that blocks the release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants