Skip to content

Conversation

@ETOgaosion
Copy link
Collaborator

Reverts #2739

For #2794 to solve all CI faults.

@ETOgaosion ETOgaosion merged commit d04c69f into main Jul 30, 2025
8 of 9 checks passed
@ETOgaosion ETOgaosion deleted the revert-2739-gen_rm branch July 30, 2025 08:56
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +102 to 106
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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.

Suggested change
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
)
]

yellowbee686 pushed a commit to yellowbee686/verl that referenced this pull request Jul 31, 2025
Juniper1021 pushed a commit to Juniper1021/verl that referenced this pull request Aug 7, 2025
whatadayG pushed a commit to whatadayG/verl that referenced this pull request Sep 5, 2025
chenjiaoAngel added a commit to chenjiaoAngel/verl that referenced this pull request Nov 14, 2025
TimurTaepov pushed a commit to giorgossideris/verl that referenced this pull request Dec 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants