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

multi-gpu ppo #2288

Merged
merged 31 commits into from
Aug 1, 2019
Merged

multi-gpu ppo #2288

merged 31 commits into from
Aug 1, 2019

Conversation

dongruoping
Copy link
Contributor

add multi-gpu ppo training

@CLAassistant
Copy link

CLAassistant commented Jul 18, 2019

CLA assistant check
All committers have signed the CLA.

@dongruoping dongruoping requested a review from harperj July 18, 2019 22:49
Copy link
Contributor

@harperj harperj left a comment

Choose a reason for hiding this comment

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

Had some in person and in-PR feedback. Let's also chat a bit about testing this.

ml-agents/mlagents/trainers/ppo/models.py Outdated Show resolved Hide resolved
ml-agents/mlagents/trainers/ppo/models.py Outdated Show resolved Hide resolved
ml-agents/mlagents/trainers/ppo/trainer.py Outdated Show resolved Hide resolved
ml-agents/mlagents/trainers/ppo/trainer.py Outdated Show resolved Hide resolved
ml-agents/mlagents/trainers/ppo/trainer.py Outdated Show resolved Hide resolved
ml-agents/mlagents/trainers/ppo/models.py Outdated Show resolved Hide resolved
@dongruoping dongruoping requested a review from harperj July 22, 2019 21:40
ml-agents/mlagents/trainers/ppo/multi_gpu_policy.py Outdated Show resolved Hide resolved
ml-agents/mlagents/trainers/ppo/multi_gpu_policy.py Outdated Show resolved Hide resolved
ml-agents/mlagents/trainers/ppo/multi_gpu_policy.py Outdated Show resolved Hide resolved
Copy link
Contributor

@harperj harperj left a comment

Choose a reason for hiding this comment

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

This is looking a lot better to my eyes. I think you'll want to at least make sure we get some unit test coverage on the core pieces of the MultiGpuPPOPolicy code-- update, average_gradients, and get_devices. It should now be easier to mock out construct_feed_dict so that you can be sure your update function behaves as you'd expect.

@dongruoping dongruoping merged commit fba7989 into develop Aug 1, 2019
@dongruoping dongruoping deleted the develop-multi-gpu branch August 2, 2019 22:01
@dongruoping dongruoping restored the develop-multi-gpu branch August 6, 2019 05:30
@dongruoping dongruoping deleted the develop-multi-gpu branch August 9, 2019 03:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants