Skip to content
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

increase default ppo batch size from 64 to 256 #405

Closed
wants to merge 1 commit into from
Closed

increase default ppo batch size from 64 to 256 #405

wants to merge 1 commit into from

Conversation

matwilso
Copy link

@matwilso matwilso commented Apr 24, 2021

PPO can work poorly with small batch sizes, so 256 is probably a better default for people than 64. Also, 256 is the old default batch size in the original stable baselines (#90 (comment)), as well SAC currently.

I thought this was a minor enough change not to require the normal PR process. But let me know if you want me to go through that.

PPO can work poorly with small batch sizes. The old default batch size in the original stable baselines was 256 (#90 (comment))

And this is also the default for SAC.
@Miffyli
Copy link
Collaborator

Miffyli commented Apr 24, 2021

Hello. Please open up an issue discussing this change. The "non-issue PRs" are only reserved for hotfixes and very small updates like documentation typo fixes. This would change the behaviour of existing codebases, so it needs to be properly discussed.

@Miffyli Miffyli added the PR template not filled Please fill the pull request template label Apr 24, 2021
@araffin
Copy link
Member

araffin commented Apr 26, 2021

Hello,

I'm not sure where you have seen that SB2 PPO had 256 as default batch size (the comment you linked was only valid for a particular example).
Make sure to read our migration guide: https://stable-baselines3.readthedocs.io/en/master/guide/migration.html
and afterward open an issue (as mentioned by @Miffyli ) if you think there is still one.

PS: please read our contributing guide and keep the PR template next time.

@matwilso matwilso closed this Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR template not filled Please fill the pull request template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants