[chore] remove unnecessary updating of _worker_names
#19
Conversation
|
trace the code more. It may be required because the constructor is called inside the same class's method update: initializes |
|
I will check if this will bring some conflicts tomorrow. Initialize the |
|
Thanks! @PeterSH6 I will also try to run some examples from my side. Would you mind giving me a pointer about the tests? |
|
@kevin85421 Thanks! We didn't open-source the tests as we haven't set up the CI in GitHub recently. I will release some relevant tests later. |
| @@ -187,6 +187,9 @@ def __init__(self, | |||
| self.ray_cls_with_init = ray_cls_with_init | |||
| self.name_prefix = get_random_string(length=6) if name_prefix is None else name_prefix | |||
|
|
|||
| if worker_names is not None: | |||
| self._worker_names = worker_names | |||
There was a problem hiding this comment.
I wonder if adding assert self._is_init_with_detached_workers between L190-191 would be better?
Your modification will not cause errors in current usage but it may be better to add this line.
|
@kevin85421 I have added some tests for Ray single-controller in #23 These test files would be useful for this PR: |
Signed-off-by: Kai-Hsun Chen <[email protected]>
|
Thank you for the review! I will spend more time on this repo after Thanksgiving. We are currently working on a PoC for veRL with Ray Compiled Graphs. In addition, we have limited access to A100s. Are there any examples that can be run on a smaller GPU machine so that I can use for dev? |
It looks like that
self._worker_namesdoesn't need to be updated.verl/single_controller/ray/base.py
Line 261 in 27b43eb