Fix pickle error in prime reward manager when prime in main_ppo.py#1296
Fix pickle error in prime reward manager when prime in main_ppo.py#1296cgpeter96 wants to merge 4 commits into
Conversation
| async def parallel_compute_score_async(evaluation_func, completions, references, tasks, extra_info=None, num_processes=64): | ||
| scores = [] | ||
| with ProcessPoolExecutor(max_workers=num_processes) as executor: | ||
| with ThreadPoolExecutor(max_workers=num_processes) as executor: |
There was a problem hiding this comment.
ThreadPoolExecutor is not a good idea since compute_score is compute intensive, python GIL prevents it to use multi cores.
There was a problem hiding this comment.
ThreadPoolExecutor is not a good idea since compute_score is compute intensive, python GIL prevents it to use multi cores.
Understood, I will update the code shortly.
|
I actually ran into the same issue as you, and I think your solution is unnecessarily complex. I also got the issue that the function is not pickleable - the issue is that the A simple solution that works for me is replacing the wrapped function with a partial over the raw function. -> No other changes are needed. Do you agree? I don't see what the point of wrapping the function is, other than passing in the |
|
Also, the function |
Of course, simple implementation is best :)
YES! You are right. The simple solution is best |
Yes! |
|
@wuxibin89 Could you given me some feedback 😊. This pr will help someone who wanna use prime reward manager in ppo/grpo training but failed. |
|
Hi @wuxibin89 @vermouth1992 ! We also stumbled in this issue with a custom reward function defined in a sidekick python file: We tried the We're calling verl trainer as follows: python -m verl.trainer.main_ppo \
custom_reward_function.path=/mnt/fs/verl/custom_function.py \
custom_reward_function.name=compute_score \
reward_model.reward_manager=prime \
...Would you have any suggestions how to fix this? |
Meeting exactly the same issue. |
|
I ran into this issue as well (even with #2239). Here is my fix for the issue which worked for me. |
|
This code seems to be inconsistent with the latest version. Are there any latest solutions to this problem at present? |
I tried your plan, but it reported an error |
fixed #1293