Replaced rllab.envs.Env with gym.Env by jonashen · Pull Request #129 · ryanjulian/rllab

jonashen · 2018-06-08T01:42:07Z

Using Zhanpeng's refactoring of normalized_env as a baseline, this PR creates two utility files (i.e. gym_env_util and gym_space_util) to convert gym spaces for use in rllab algorithms.

Ref: #85

zhanpenghe · 2018-06-08T12:07:40Z

+        return special.from_onehot(obs)
+    elif isinstance(space, gym.spaces.Tuple):
+        return np.concatenate(
+            [gym_space_unflatten(xi, c) for c, xi in zip(space.spaces, obs)])


Did you test the unflatten of Tuple? It should take account of the dims of components of the tuples and reshape the flatten obs. Unflatten is a bit more complicated than flatten.

zhanpenghe · 2018-06-08T12:11:15Z

+        return special.from_onehot_n(obs)
+    elif isinstance(space, gym.spaces.Tuple):
+        return np.concatenate(
+            [gym_space_unflatten_n(xi, c) for c, xi in zip(space.spaces, obs)])


Same as unflatten. Please check the unflatten_n of rllab.spaces.product

zhanpenghe · 2018-06-08T12:14:13Z

+]
+
+
+def action_dim(env):


This can be rename to action_flat_dim. With flat_dim() of spaces, I don't think this function is necessary.

zhanpenghe · 2018-06-08T16:30:42Z

+
+
+def log_diagnostics(env, paths):
+    pass


This function is different for different environments. It's not a good practice to implement one function for all envs because there could be too many of them.

zhanpenghe · 2018-06-08T16:32:34Z

+from rllab.misc import ext
+from rllab.misc import special
+from rllab.misc.overrides import overrides
+


Please remove imports that is not used.

…tion_dim to action_flat_dim

zhanpenghe

Please also remove files that used GymEnv in examples since GymEnv would not be used anymore.

zhanpenghe · 2018-06-09T13:00:44Z

+
+
+def log_diagnostics(space, paths):
+    pass


Please remove useless function.

zhanpenghe · 2018-06-09T13:03:26Z

+
+def components(space):
+    if isinstance(space, gym.spaces.Tuple):
+        return self.spaces


This is actually only for Tuple. Using space.spaces looks cleaner than me.

zhanpenghe

Please double check your codes when copying from other files.

zhanpenghe · 2018-06-09T18:12:33Z

+        return special.from_onehot(obs)
+    elif isinstance(space, gym.spaces.Tuple):
+        dims = [flat_dim(c) for c in space.spaces]
+        flat_xs = np.split(x, np.cumsum(dims)[:-1])


Please do not just copy paste codes.. Variable x is not declared at all.

zhanpenghe · 2018-06-09T18:13:08Z

+        return special.from_onehot_n(obs)
+    elif isinstance(space, gym.spaces.Tuple):
+        dims = [flat_dim(c) for c in self.spaces]
+        flat_xs = np.split(xs, np.cumsum(dims)[:-1], axis=-1)


Same as unflatten(space, obs).. also line 107, there is not self here because is not an object.

zhanpenghe · 2018-06-09T18:16:09Z

+def sample(space):
+    if isinstance(space, gym.spaces.Tuple):
+        return tuple(x.sample() for x in self.spaces)
+    else:


There is not self in this method.

ryanjulian · 2018-06-09T19:36:59Z

@@ -1,38 +0,0 @@
-from rllab.algos import TRPO


why remove this file?

ryanjulian · 2018-06-09T19:37:03Z

@@ -1,38 +0,0 @@
-from rllab.algos import TRPO


why remove this file?

ryanjulian · 2018-06-09T19:37:15Z

@@ -1,38 +0,0 @@
-from rllab.algos import TRPO


why remove this file?

ryanjulian · 2018-06-09T19:37:24Z

@@ -1,40 +0,0 @@
-# This doesn't work. After 150 iterations still didn't learn anything.


why remove this file?

ryanjulian · 2018-06-09T19:37:32Z

@@ -1,38 +0,0 @@
-from rllab.algos import TRPO


why remove this file?

ryanjulian · 2018-06-09T19:42:28Z

@@ -1,48 +0,0 @@
-from rllab.algos import TRPO


why remove this file?

ryanjulian · 2018-06-09T19:42:36Z

@@ -1,42 +0,0 @@
-from rllab.baselines import LinearFeatureBaseline


why remove this file?

ryanjulian · 2018-06-09T19:46:01Z

@@ -0,0 +1,37 @@
+import gym


this can be located in rllab.envs.util / rllab/envs/util.py

No need for the long import path.

ryanjulian · 2018-06-09T19:48:26Z

@@ -0,0 +1,141 @@
+import gym


these probably also belong in rllab/envs/util.py

ryanjulian · 2018-06-09T19:49:36Z

@@ -1,3 +1,4 @@
+import gym


what is the relationship between this file and #125 ?

This file will be replaced by #125, but the utility functions located in #125 are relocated here in rllab.envs.util. I will remove edits to this file.

ryanjulian · 2018-06-09T21:43:47Z



-_Step = collections.namedtuple("Step", ["observation", "reward", "done", "info"])
+_Step = collections.namedtuple("Step",


there don't appear to be any meaningful changes in this file?

ryanjulian

I know it's painful, but can you please remove all YAPF reformatting for non-new files?

YAPF formatting makes it difficult to figure out what changed in large changes like this.

ryanjulian · 2018-06-11T04:53:27Z

Please reopen this PR against https://github.com/rlworkgroup/garage

jonashen requested review from eric-heiden, ryanjulian and zhanpenghe June 8, 2018 01:42

jonashen force-pushed the gym_env branch from 1a9703b to 6f0d7e7 Compare June 8, 2018 01:44

jonashen added 3 commits June 7, 2018 18:45

Added gym conversion utility functions

6d2028e

Refactored rllab.Env to gym.Env

9822c61

Removed unused rllab.Env imports

f4d4448

jonashen force-pushed the gym_env branch from 6f0d7e7 to f4d4448 Compare June 8, 2018 01:46

zhanpenghe requested changes Jun 8, 2018

View reviewed changes

jonashen added 3 commits June 8, 2018 11:27

Updated Tuple space_utils, created new util directory, renamed all ac…

616ae5f

…tion_dim to action_flat_dim

Remove unused action_flat_dim import

b8210eb

Fixed CarParkingEnv action_flat_dim error

d541a0b

jonashen requested a review from zhanpenghe June 8, 2018 20:56

zhanpenghe requested changes Jun 9, 2018

View reviewed changes

zhanpenghe mentioned this pull request Jun 9, 2018

Normalized gym env #125

Closed

jonashen added 2 commits June 9, 2018 12:50

Refactored space_util

57a9804

Removed all references/usage of GymEnv

32bef7b

jonashen force-pushed the gym_env branch from 0962e0e to 32bef7b Compare June 9, 2018 19:50

ryanjulian reviewed Jun 9, 2018

View reviewed changes

jonashen added 3 commits June 9, 2018 13:05

Added gym examples

5189610

Condensed env_util and space_util into rllab.envs.util

89514a7

Refactored GymEnv to gym.make

80d1020

ryanjulian reviewed Jun 9, 2018

View reviewed changes

ryanjulian requested changes Jun 9, 2018

View reviewed changes

More gym examples

c88a639

zhanpenghe mentioned this pull request Jun 10, 2018

Cleanup normalized env #131

Closed

ryanjulian mentioned this pull request Jun 11, 2018

Cleanup normalized env rlworkgroup/garage#2

Closed

Removed YAPF reformatting

42bb6b2

		@@ -1,40 +0,0 @@
		# This doesn't work. After 150 iterations still didn't learn anything.

		@@ -1,42 +0,0 @@
		from rllab.baselines import LinearFeatureBaseline



		_Step = collections.namedtuple("Step", ["observation", "reward", "done", "info"])
		_Step = collections.namedtuple("Step",

		]


		def action_dim(env):

Conversation

jonashen commented Jun 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhanpenghe Jun 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhanpenghe left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhanpenghe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanjulian left a comment

Choose a reason for hiding this comment

Uh oh!

ryanjulian commented Jun 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jonashen commented Jun 8, 2018 •

edited

Loading

zhanpenghe Jun 8, 2018 •

edited

Loading

zhanpenghe left a comment •

edited

Loading