Skip to content

Conversation

jon-tow
Copy link
Collaborator

@jon-tow jon-tow commented Nov 10, 2022

This PR decouples the PPO loss function from the AcceleratePPOModel class for general reuse following @cat-state 's work in #75 . The following updates are also provided:

  • Adds the unbiased KL-div estimates following http://joschu.net/blog/kl-approx.html
  • Removes repeated initialization of self.kl_ctl in accelerate_ppo_models constructor.
  • Removes utils.modeling.clip_by_value as this functionality was added to PyTorch nearly 2 years ago.
  • Introduces more PPO statistics to the logger for greater debugging information.

Test Run: https://wandb.ai/jon-tow/ppo-test

@jon-tow jon-tow marked this pull request as ready for review November 10, 2022 22:24
@LouisCastricato
Copy link
Contributor

This looks great on my side, @Dahoas im open to merging this tonight if this is ok for you too?

Copy link
Collaborator

@cat-state cat-state left a comment

Choose a reason for hiding this comment

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

LGTM!

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