-
Notifications
You must be signed in to change notification settings - Fork 95
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
chore: PPO system cleanup #1124
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.
Thank you @sash-a! I think these are some very needed changes. I just have a few suggestions and questions. Any comment applies to all systems.
todo: same fixes for sable and switch sable to using the PPOTransition |
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.
LGTM! Let's just wait for the benchmark before we merge this 🙏
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.
🏆
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.
Thanks @sash-a 🙌
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.
🌟
- chore: remove jax.tree_map - Merge branch develop into chore/ppo-system-cleanup - fix: update ret_output shape (#1147) - Merge branch develop into chore/ppo-system-cleanup - chore: minor comment changes - fix: integration test workflow (#1145) - Merge branch develop into chore/ppo-system-cleanup - Merge branch develop into chore/ppo-system-cleanup - chore: renaming loss_X -> X_loss - fix: updated sebulba - Merge branch develop into chore/ppo-system-cleanup - chore: remove expired link to anakin notebook - chore: add back advanced usage in examples - Merge branch develop into chore/ppo-system-cleanup - feat: sable cleanup - chore: merge dev - chore: unify value/critic loss naming - refactor: loss_actor -> actor_loss - Merge branch develop into chore/ppo-system-cleanup - chore: pre-commit - feat: mat system clean up - chore: remove advanced usage - Merge branch develop into chore/ppo-system-cleanup - feat: ff mappo system clean up - feat: ff ippo system clean up - feat: rec mappo system clean up - feat: rec ippo system clean up Co-authored-by: Ruan de Kock <[email protected]> Co-authored-by: Louay-Ben-nessir <[email protected]> Co-authored-by: Arnol Fokam <[email protected]>
- chore: remove jax.tree_map - Merge branch develop into chore/ppo-system-cleanup - fix: update ret_output shape (#1147) - Merge branch develop into chore/ppo-system-cleanup - chore: minor comment changes - fix: integration test workflow (#1145) - Merge branch develop into chore/ppo-system-cleanup - Merge branch develop into chore/ppo-system-cleanup - chore: renaming loss_X -> X_loss - fix: updated sebulba - Merge branch develop into chore/ppo-system-cleanup - chore: remove expired link to anakin notebook - chore: add back advanced usage in examples - Merge branch develop into chore/ppo-system-cleanup - feat: sable cleanup - chore: merge dev - chore: unify value/critic loss naming - refactor: loss_actor -> actor_loss - Merge branch develop into chore/ppo-system-cleanup - chore: pre-commit - feat: mat system clean up - chore: remove advanced usage - Merge branch develop into chore/ppo-system-cleanup - feat: ff mappo system clean up - feat: ff ippo system clean up - feat: rec mappo system clean up - feat: rec ippo system clean up Co-authored-by: Louay-Ben-nessir <[email protected]> Co-authored-by: Arnol Fokam <[email protected]> Co-authored-by: Ruan de Kock <[email protected]>
What?
Did a couple small clean ups to the PPO systems that are long overdue:
@RuanJohn should we add this for MAT?