-
Notifications
You must be signed in to change notification settings - Fork 45
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: uniform sac network sizes #145
Conversation
… networks. dads and diayn too
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## develop #145 +/- ##
===========================================
+ Coverage 92.28% 92.29% +0.01%
===========================================
Files 116 116
Lines 6763 6772 +9
===========================================
+ Hits 6241 6250 +9
Misses 522 522
📣 We’re building smart automated test selection to slash your CI/CD build times. 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.
Notebooks might need to be updated as well.
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.
Looks good 👍
This PR is to try to make the Deep RL algorithms more uniform. It allows the SAC, DIAYN and DADS implementation to have separate policy (actor) and critic architectures, as is already the case in the TD3 implementation. This allows more flexibility and is needed sometimes for comparison with QD algorithms which do not necessarily have a critic which needs a bigger size while the actor can be smaller.
This PR changes:
Checks
Future improvements
for DADS and DIAYN there is also the discriminator or dynamics architecture to expose. For now it is set to the critic architecture