[RLlib] Add custom_resources_per_learner config#63303
Conversation
Mirror of `custom_resources_per_env_runner`: lets users attach custom Ray resource requirements to each Learner worker. Plumbed into `learner_group.py`'s `resources_per_learner` dict so the custom resources are claimed alongside CPU/GPU when Ray Train schedules Learners. Signed-off-by: Artur Niederfahrenhorst <artur@anyscale.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the custom_resources_per_learner configuration to AlgorithmConfig, allowing users to allocate custom Ray resources to learner workers. The changes include updating the LearnerGroup to incorporate these resources and adding a new test suite to verify the functionality. Review feedback recommends adding a ValueError to prevent users from including "CPU" or "GPU" in the custom resources dictionary and adjusting the resource dictionary construction to ensure explicit CPU and GPU settings are not overwritten.
ArturNiederfahrenhorst
added
rllib
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Signed-off-by: Artur Niederfahrenhorst <artur@anyscale.com>
pseudo-rnd-thoughts
changed the title
custom_resources_per_learner config knobcustom_resources_per_learner config
pseudo-rnd-thoughts
left a comment
pseudo-rnd-thoughts
left a comment
There was a problem hiding this comment.
Overall looks good, just two small requests
| or "GPU" in custom_resources_per_learner | ||
| ): | ||
| raise ValueError( | ||
| "Do not include 'CPU' or 'GPU' in " |
There was a problem hiding this comment.
Include the custom_resources_per_learner in the error message
| if num_gpus_per_learner is not NotProvided: | ||
| self.num_gpus_per_learner = num_gpus_per_learner | ||
| if custom_resources_per_learner is not NotProvided: | ||
| if custom_resources_per_learner and ( |
There was a problem hiding this comment.
Shouldn't the first part be isinstance(custom_resources_per_learner, dict) or remove it
- Drop truthiness guard on validation check so non-dict inputs raise clearly instead of silently passing - Include the offending value in the ValueError message - Add None guard in utils.py to match learner_group.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 0773167. Configure here.
| if custom_resources_per_learner is not NotProvided: | ||
| if ( | ||
| "CPU" in custom_resources_per_learner | ||
| or "GPU" in custom_resources_per_learner |
There was a problem hiding this comment.
Missing dict type guard before in operator check
Medium Severity
The custom_resources_per_learner parameter is typed Optional[Dict[str, float]], so None is a valid input. When None is passed, None is not NotProvided evaluates to True, causing the code to execute "CPU" in None, which raises an unhelpful TypeError instead of a clear validation error. An isinstance(custom_resources_per_learner, dict) guard is needed before the in check, as the PR reviewer also flagged.
Reviewed by Cursor Bugbot for commit 0773167. Configure here.
2 participants
Description
Mirror of
custom_resources_per_env_runner: lets users attach custom Ray resource requirements to each Learner worker. Plumbed intolearner_group.py'sresources_per_learnerdict so the custom resources are claimed alongside CPU/GPU when Ray Train schedules Learners.