Skip to content

Revert "[recipe] feat: Add sleep/wakeup mode for gen rm vllm service and add tqdm showing process" - #2813

Merged
ETOgaosion merged 1 commit into
mainfrom
revert-2739-gen_rm
Jul 30, 2025
Merged

ETOgaosion merged 1 commit into
mainfrom
revert-2739-gen_rm

Conversation

@ETOgaosion

Copy link
Copy Markdown
Collaborator

Reverts #2739

For #2794 to solve all CI faults.

@ETOgaosion
ETOgaosion merged commit d04c69f into main Jul 30, 2025
@ETOgaosion
ETOgaosion deleted the revert-2739-gen_rm branch July 30, 2025 08:56

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

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
vyomakesh0728 added a commit to vyomakesh0728/verl that referenced this pull request Jan 22, 2026
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.

2 participants