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

Add automatic restart of failing processes in AsyncVectorEnv #1602

Closed

Conversation

tristandeleu
Copy link
Contributor

@tristandeleu tristandeleu commented Jul 12, 2019

This PR adds an option to automatically restart processes in AsyncVectorEnv if one of the processes failed, up to a max_retries number of retries. The goal is to avoid the vectorized environment to fail if there is an issue with one environment. Failures may happen, and it would can be frustrating to have a whole run killed by some failing environment.

This PR has gone through a significant rework since #1513, to account for the more recent changes added, so it would probably need a more careful review. I think it could be a nice feature to have, but since I have never faced that situation of a failing environment myself, I would be happy to have any feedback and discuss this change!

@christopherhesse
Copy link
Contributor

Thanks for the PR! I'd like to wait until there are some users of this feature before merging it though. We haven't needed this with our VecEnv implementation, and it's not clear to me that this should exist here instead of as a wrapper.

@tristandeleu
Copy link
Contributor Author

I agree, ideally it would make more sense to have this feature as a wrapper around AsyncVectorEnv. But this requires a number of backend changes (like the _restart function in the workers, obs_buffer and ctx as properties of AsyncVectorEnv), which would a priori be outside of the scope of what a wrapper would be able to do.

That being said, I also agree that we should wait before merging this PR. It is unclear that it would actually be useful at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants