Repository navigation
Revert "[recipe] feat: Add sleep/wakeup mode for gen rm vllm service and add tqdm showing process" - #2813
Conversation
…and add…" This reverts commit 76298ad.
There was a problem hiding this comment.
Code Review
This pull request reverts a previous feature related to vLLM service sleep/wakeup modes, presumably to fix CI issues. The changes correctly remove the feature's implementation and documentation. My review focuses on the resulting code within the diff. I've identified an opportunity to improve the conciseness and maintainability of the task submission logic in reward_function.py by using a more idiomatic Python construct. The provided suggestion will make the code cleaner without altering its behavior.
| for data_source, solution_str, ground_truth, extra_info in zip( | ||
| data_sources, solution_strs, ground_truths, extra_infos, strict=True | ||
| ): | ||
| future = executor.submit(compute_score, data_source, solution_str, ground_truth, extra_info, index) | ||
| time.sleep(0.001 * random.random()) | ||
| future = executor.submit(compute_score, data_source, solution_str, ground_truth, extra_info) | ||
| futures.append(future) |
There was a problem hiding this comment.
The for loop used to submit tasks to the ThreadPoolExecutor is unnecessarily verbose. This can be refactored into a more concise and idiomatic list comprehension.
This change improves readability and maintainability by replacing a multi-line loop with a single, expressive statement, which is a common pattern in modern Python for this type of task.
| for data_source, solution_str, ground_truth, extra_info in zip( | |
| data_sources, solution_strs, ground_truths, extra_infos, strict=True | |
| ): | |
| future = executor.submit(compute_score, data_source, solution_str, ground_truth, extra_info, index) | |
| time.sleep(0.001 * random.random()) | |
| future = executor.submit(compute_score, data_source, solution_str, ground_truth, extra_info) | |
| futures.append(future) | |
| futures = [ | |
| executor.submit(compute_score, data_source, solution_str, ground_truth, extra_info) | |
| for data_source, solution_str, ground_truth, extra_info in zip( | |
| data_sources, solution_strs, ground_truths, extra_infos, strict=True | |
| ) | |
| ] |
…and add tqdm showing process" (verl-project#2813) Reverts verl-project#2739 For verl-project#2794 to solve all CI faults.
…and add tqdm showing process" (verl-project#2813) Reverts verl-project#2739 For verl-project#2794 to solve all CI faults.
…and add tqdm showing process" (verl-project#2813) Reverts verl-project#2739 For verl-project#2794 to solve all CI faults.
…and add tqdm showing process" (verl-project#2813) Reverts verl-project#2739 For verl-project#2794 to solve all CI faults.
…and add tqdm showing process" (verl-project#2813) Reverts verl-project#2739 For verl-project#2794 to solve all CI faults.
…and add tqdm showing process" (verl-project#2813) Reverts verl-project#2739 For verl-project#2794 to solve all CI faults.
Reverts #2739
For #2794 to solve all CI faults.