[Core] respect bundle index when task is scheduled with pg with no resources. #43448
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
February 27, 2024 09:13
jjyao
reviewed
Feb 27, 2024
jjyao
approved these changes
Feb 27, 2024
python/ray/_private/utils.py
Outdated
| if result and len(result.groups()) == 2: | ||
| result = PLACEMENT_GROUP_INDEXED_BUNDLED_RESOURCE_PATTERN.match(key) | ||
| if result and len(result.groups()) == 3: | ||
| # This should be already skipped from the logic above. |
There was a problem hiding this comment.
This comment should be removed
| @@ -203,6 +203,11 @@ std::unordered_map<std::string, double> AddPlacementGroupConstraint( | |||
| auto bundle_key = | |||
| FormatPlacementGroupResource(kBundle_ResourceLabel, placement_group_id, -1); | |||
| new_resources[bundle_key] = 0.001; | |||
| if (bundle_index >= 0) { | |||
There was a problem hiding this comment.
Actually I'm wondering if we should only add bundle resource when normal resources are empty.
There was a problem hiding this comment.
We should add index resource in case users want to use bundle_index
8 tasks
can-anyscale
pushed a commit
that referenced
this pull request
Feb 28, 2024
This PR automatically merge the trainer bundle with the rank 0 worker bundle, so that the trainer and rank 0 worker can always colocate on the same node.
### Benefits:
- Enables users to specify additional resources for rank 0 worker.
- Always colocate trainers and rank 0 workers together to make the scheduling behavior deterministic.
### Major changes:
#### 1. Merge trainer bundle and the first worker bundle.
Specifically, we build a placement groups with bundles `[{}, {trainer+worker}, {worker}, ..., {worker}]`, and schedule the `TrainTrainable` with the first non-empty bundle. When assigning worker ranks, we designate the worker with the smallest GPU ID on the same node as the trainer to be rank 0.
#### 2. Set `num_workers=1` by default in `ScalingConfig`.
Previously, setting `num_workers` to `None` resulted launching a single `TrainTrainable` with zero workers. It no longer applies to the current Ray Train, as all Trainers now require at least one worker to execute the `train_func`.
Additionally, this approach led to undefined behaviors during the merging and separation of the first bundle. To ensure the consistent behavior, we have now set the default value of `num_workers` to 1.
#### 3. Forbid using `ScalingConfig` with `tune.with_resources`.
`ScalingConfig` should be a Ray Train only utility and it's should not be used for Tune Trainables. For example, it doesn't make sense to provide ScalingConfig for a function trainable, since there's no trainer and worker concepts for it.
Passed Release Test：https://buildkite.com/ray-project/release/builds/9650#018dee6e-e3ce-4376-9f3d-5ad7e250e513
## Related PRs:
The below two PRs enabled that the actors with empty resources can be launched on the node of a specific bundle in placement group.
- #43269
- #43448
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: Yunxuan Xiao <xiaoyunxuan1998@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
#43269 starts respecting pg when resources are not specified for tasks. But it still doesn't respect bundle index. This PR fixes the issue by always including bundle index formatted resources if bundle index is specified.
This PR also adds an additional unit test to check observability API not displaying formatted resources
Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.