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

Make SubprocessEnvManager take asynchronous steps #2265

Merged
merged 5 commits into from
Jul 19, 2019

Conversation

harperj
Copy link
Contributor

@harperj harperj commented Jul 15, 2019

SubprocessEnvManager takes steps synchronously to reproduce old
behavior, meaning all parallel environments will need to wait for
the slowest environment to take a step. If some steps take much
longer than others, this can lead to a substantial overall slowdown
in practice. We've seen extreme cases where we see almost a 2x
speedup from using asynchronous stepping, with no downside for our
faster environments.

This PR changes the SubprocessEnvManager to use async stepping.
This means on the "step" call the environment manager will enqueue
step requests to workers, and then only wait until at least one
step has been completed before returning.

Testing

Reacher environment on GCP with 8 parallel environments. 15000 steps. 16.5% improvement in overall time for Async EnvManager, comparable reward.

Walker environment on GCP with 8 parallel environments. 30000 steps. 14.2% improvement in overall time for Async EnvManager, comparable reward.

@harperj harperj requested a review from ervteng July 15, 2019 18:47
Copy link
Contributor

@ervteng ervteng left a comment

Choose a reason for hiding this comment

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

Looks good! May want to run the example environments (on the cloud) through it just in case

from multiprocessing.connection import Connection
from queue import Empty as EmptyQueue
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe call this EmptyQueueException? bad naming in the stdlib :/

Copy link
Contributor

@chriselion chriselion left a comment

Choose a reason for hiding this comment

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

Looks good after fixing up the step_queue on reset.

Jonathan Harper added 4 commits July 19, 2019 14:19
SubprocessEnvManager takes steps synchronously to reproduce old
behavior, meaning all parallel environments will need to wait for
the slowest environment to take a step.  If some steps take much
longer than others, this can lead to a substantial overall slowdown
in practice.  We've seen extreme cases where we see almost a 2x
speedup from using asynchronous stepping, with no downside for our
faster environments.

This PR changes the SubprocessEnvManager to use async stepping.
This means on the "step" call the environment manager will enqueue
step requests to workers, and then only wait until at least one
step has been completed before returning.
@harperj harperj merged commit 7c374f2 into develop Jul 19, 2019
mantasp added a commit that referenced this pull request Jul 22, 2019
* develop: (69 commits)
  Add different types of visual encoder (nature cnn/resnet)
  Make SubprocessEnvManager take asynchronous steps (#2265)
  update mypy version
  one more unused
  remove unused variables
  Fix respawn part of BananaLogic (#2277)
  fix whitespace and line breaks
  remove codacy (#2287)
  Ported documentation from other branch
  tennis reset parameter implementation ported over
  Fixed the default value to match the value in the docs
  two soccer reset parameter implementation ported over
  3D ball reset parameter implementation ported over
  3D ball reset parameter implementation ported over
  Relax the cloudpickle version restriction (#2279)
  Fix get_value_estimate and buffer append (#2276)
  fix lint checks
  Add Unity command line arguments
  Swap 0 set and reward buffer append (#2273)
  GAIL and Pretraining (#2118)
  ...
@awjuliani awjuliani deleted the develop-async-env-manager branch July 23, 2019 20:20
@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.

3 participants