Thanks for reporting it. Would you like to open a PR to fix it?

@kechunFIVE, @qgallouedec, @dame-cell Seems that current code is inspired by https://iclr-blogposts.github.io/2024/blog/the-n-implementation-details-of-rlhf-with-ppo/, section General implementation details, 4.2. Authors tried to recreate results from early OpenAI work, but they say:

Note that in a more recent codebase https://github.com/openai/summarize-from-feedback, OpenAI does stop sampling when encountering EOS token (summarize_from_feedback/utils/experiment_helpers.py#L19). However in this work we aim to do a 1:1 replication, so we align the setting that could keep sampling even eos_token is encountered

Reading PPOTrainer implementation, it seems that stop_token/stop_token_id arguments should control when to stop generation:

• When stop_token is set to eos value, everything that is after EOS is padded and therefore ignored in calculations (using masks). In that case, keeping generating after all sequences have EOS seems just wasting of time and resources. I've checked that between stopping on EOS and generating until max length, loss values are almost exact and minimal differences of the order of 1e-8 are caused by masked calculations.
• When stop_token_id is set, sequences get padded after this token, so generation should stop there, not model's EOS.
• The case when stop_token != None and stop_token != 'eos' are not supported, so they should probably raise an exception.