-
Notifications
You must be signed in to change notification settings - Fork 341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FIRE sampling added. #58
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review and thanks for the contribution! Would you mind introducing a new class inheriting vllmRollout, add the custom sampling logics? This way it'd be easier to maintain the codebase
Hi, Sorry for the delay as I find it extremely challenging to update the code with the latest version of the verl as many things have changed and I am not able to access the original test environment again. I have updated the code logic following the suggestion from @eric-haibin-lin above, and also merge the latest commits. Please take another round of review and let me know if there is any further suggestions. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank u. could you fix the errors reported from CI?
Hi, Thanks for the info. Again, I am not so familiar with the CI system, so I am wondering is the current status ready to go after my last commit? Or is there anything I still need work on as the only failing check I see seems to be caused by random shutdown from the server. |
Add FIRE sampling from paper Flaming-hot Initiation with Regular Execution Sampling for Large Language Models (https://arxiv.org/abs/2410.21236) to the PPO trainer loop as an additional option to turn on.