[RLlib] Don't add a cpu to bundle for learner when using gpu #35529
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
Signed-off-by: Avnish <[email protected]>
solves ray-project#35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]>
avnishn
requested review from
sven1977,
gjoliver,
ArturNiederfahrenhorst,
smorad,
maxpumperla,
kouroshHakha and
krfricke
as code owners
Signed-off-by: avnishn <[email protected]>
Signed-off-by: avnishn <[email protected]>
|
@avnishn Thanks man! Can you also make sure that a pick lands? |
kouroshHakha
approved these changes
May 19, 2023
Comment on lines
+3243
to
+3247
| num_cpus_per_learner_worker=( | ||
| self.num_cpus_per_learner_worker | ||
| if not self.num_gpus_per_learner_worker | ||
| else 0 | ||
| ), |
There was a problem hiding this comment.
qq: this is not needed right? cause self.num_cpus_per_learner_worker will be zero if self.num_gpus_per_learner_worker > 0. If this is not the case, we already raise errors.
There was a problem hiding this comment.
yeah I think I just added this in here before I added the error. We don't technically need this Ill remove it if I need to do anything to get the release tests to run.
|
Don't merge just yet For whatever reason, I haven't gotten the release tests to run, Like its not finding the name of the test.... I suspect I did something wrong while launching the test. |
|
ok let me know when. 👍 |
gjoliver
reviewed
May 19, 2023
Signed-off-by: Avnish <[email protected]>
…avoid hanging experiment Signed-off-by: Avnish <[email protected]>
Signed-off-by: Avnish <[email protected]>
Signed-off-by: Avnish <[email protected]>
|
https://buildkite.com/ray-project/release-tests-pr/builds/39400#0188443c-35b2-4d89-aaa3-9b8f8b442826 release tests are passing, this is ready to merge @kouroshHakha |
There was a problem hiding this comment.
There are some non-catchable conflicts with this PR #35573. Please merge master and use the new Learner API. Also not sure if the changes to learners are needed in this PR.
…fy_resource_request_multi_gpu_learner
Signed-off-by: Avnish <[email protected]>
…efficiently and finish training faster Signed-off-by: avnishn <[email protected]>
|
https://buildkite.com/ray-project/release-tests-pr/builds/39400#_ The release tests are passing, but one of the multi gpu test examples is taking a really long time to run because of bad resource utilization, so I changed its resource utilization (changed batch and minibatch sizes, and increased number of rollout workers since we have 48 on the machine) |
Signed-off-by: avnishn <[email protected]>
Signed-off-by: avnishn <[email protected]>
Signed-off-by: Avnish <[email protected]>
Signed-off-by: Avnish <[email protected]>
…fy_resource_request_multi_gpu_learner
Signed-off-by: Avnish <[email protected]>
|
the rllib multi-gpu test that failed is flakey. I have verified by running repro ci and running the test myself that it passes. |
avnishn
added a commit
to avnishn/ray
that referenced
this pull request
May 23, 2023
…ject#35529) solves ray-project#35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]>
8 tasks
ArturNiederfahrenhorst
pushed a commit
that referenced
this pull request
May 24, 2023
…#35676) solves #35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]>
scv119
pushed a commit
to scv119/ray
that referenced
this pull request
Jun 16, 2023
…ject#35529) solves ray-project#35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]>
arvind-chandra
pushed a commit
to lmco/ray
that referenced
this pull request
Aug 31, 2023
…ject#35529) solves ray-project#35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]> Signed-off-by: e428265 <[email protected]>
glennmoy
pushed a commit
to beacon-biosignals/ray
that referenced
this pull request
Sep 26, 2023
…ject#35529) (ray-project#35676) solves ray-project#35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]>
glennmoy
pushed a commit
to beacon-biosignals/ray
that referenced
this pull request
Sep 26, 2023
…ject#35529) (ray-project#35676) solves ray-project#35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]>
glennmoy
pushed a commit
to beacon-biosignals/ray
that referenced
this pull request
Sep 26, 2023
…ject#35529) (ray-project#35676) solves ray-project#35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]>
glennmoy
pushed a commit
to beacon-biosignals/ray
that referenced
this pull request
Sep 26, 2023
…ject#35529) (ray-project#35676) solves ray-project#35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]>
glennmoy
pushed a commit
to beacon-biosignals/ray
that referenced
this pull request
Sep 26, 2023
…ject#35529) (ray-project#35676) solves ray-project#35409 Prevent fragmentation of resources by not placing gpus with cpus in bundles for the learner workers, making it so that an actor that requires only cpu does not potentially take a bundle that has both a cpu and gpu. The long term fix will be to allow the specification of placement group bundle index via tune and ray train. Signed-off-by: avnishn <[email protected]>
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.
solves #35409
Prevent fragmentation of resources by not placing gpus
with cpus in bundles for the learner workers, making it
so that an actor that requires only cpu does not
potentially take a bundle that has both a cpu and gpu.
The long term fix will be to allow the specification
of placement group bundle index via tune and ray train.
Signed-off-by: avnishn [email protected]
Why are these changes needed?
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.