OpenAI gym conversion given a gym instance #12
Conversation
| @@ -56,7 +56,7 @@ def __call__(self, count): | |||
|
|
|||
|
|
|||
| class GymEnv(Env, Serializable): | |||
| def __init__(self, env_name, record_video=True, video_schedule=None, log_dir=None, record_log=True, | |||
| def __init__(self, env, record_video=True, video_schedule=None, log_dir=None, record_log=True, | |||
There was a problem hiding this comment.
Does this work when you use run_experiment_lite() to run your algorithm (e.g. https://github.com/ryanjulian/rllab/blob/integration/examples/trpo_cartpole_pickled.py)?
I'm afraid they might have used strings to ensure that the GymEnv is Serializable
| @@ -86,7 +90,10 @@ def __init__(self, env_name, record_video=True, video_schedule=None, log_dir=Non | |||
| logger.log("observation space: {}".format(self._observation_space)) | |||
| self._action_space = convert_gym_space(env.action_space) | |||
| logger.log("action space: {}".format(self._action_space)) | |||
| self._horizon = env.spec.tags['wrapper_config.TimeLimit.max_episode_steps'] | |||
There was a problem hiding this comment.
Found the root cause of this issue here:
https://github.com/openai/gym/blob/master/gym/envs/registration.py#L48
So it seems like the real fix here is to bump the OpenAI gym version and use env.max_episode_steps. My local repo uses the latest version of gym, so I don't think bumping it (in environment.yml) should cause problems.
| @@ -65,7 +65,11 @@ def __init__(self, env_name, record_video=True, video_schedule=None, log_dir=Non | |||
| log_dir = os.path.join(logger.get_snapshot_dir(), "gym_log") | |||
| Serializable.quick_init(self, locals()) | |||
|
|
|||
| env = gym.envs.make(env_name) | |||
| if isinstance(env, str): | |||
There was a problem hiding this comment.
dispatching on argument type is a Bad Idea.
a couple strategies
- pass a class and args/kwargs for the constructor of that class (e.g. env_cls=None, env_args=[], env_kwargs=dict())
- provide a special constructor function (e.g. GymEnv.fromEnv), probably better
There was a problem hiding this comment.
I stand corrected. I much closer at this, and this is pretty much the only way to do it while keeping backwards compatibility and not copying a ton of code.
| if hasattr(env.spec, 'tags') and 'wrapper_config.TimeLimit.max_episode_steps' in env.spec.tags: | ||
| self._horizon = env.spec.tags['wrapper_config.TimeLimit.max_episode_steps'] | ||
| else: | ||
| self._horizon = 1000 |
There was a problem hiding this comment.
As far as I can tell, in updated version of gym, every env should have attribute env.max_episode_steps and this default should be unnecessary.
There was a problem hiding this comment.
Apparently the upgrade is not hard.
|
Related to #5 |
No description provided.