Skip to content

Env wrapper for RL2#1175

Merged
ahtsan merged 3 commits intomasterfrom
rl2env
Mar 3, 2020
Merged

Env wrapper for RL2#1175
ahtsan merged 3 commits intomasterfrom
rl2env

Conversation

@ahtsan
Copy link
Copy Markdown
Contributor

@ahtsan ahtsan commented Feb 17, 2020

No description provided.

@ahtsan ahtsan requested review from a team, krzentner and ryanjulian February 17, 2020 23:08
@ahtsan ahtsan requested a review from a team as a code owner February 17, 2020 23:08
@ghost ghost removed their request for review February 17, 2020 23:08
Comment thread src/garage/envs/rl2_env.py Outdated
action_flat_dim = np.prod(env.action_space.shape)
if self._max_obs_dim is not None:
obs_flat_dim = self._max_obs_dim
return gym.spaces.Box(low=-np.inf,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not just call akro.Box here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated


Args:
env (gym.Env): An env that will be wrapped.
max_obs_dim (int): Maximum observation dimension in the environments
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a needed feature to actually use ML45, but I think it probably belongs in a separate wrapper. We can address that in a later PR after submitting this.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 17, 2020

Codecov Report

Merging #1175 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1175      +/-   ##
=========================================
+ Coverage   87.95%     88%   +0.04%     
=========================================
  Files         183     185       +2     
  Lines        8711    8743      +32     
  Branches     1107    1110       +3     
=========================================
+ Hits         7662    7694      +32     
  Misses        853     853              
  Partials      196     196
Impacted Files Coverage Δ
src/garage/tf/algos/rl2/rl2env.py 100% <100%> (ø)
src/garage/tf/algos/rl2/__init__.py 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56df57e...04f06c6. Read the comment docs.

@ahtsan ahtsan requested a review from a team February 17, 2020 23:55
@ghost ghost requested review from taohnouaccountb and removed request for a team February 17, 2020 23:55
@ahtsan ahtsan requested review from naeioi and removed request for taohnouaccountb February 17, 2020 23:55
@ryanjulian ryanjulian changed the title Env wrapper for rl2 Env wrapper for RL2 Mar 2, 2020
Comment thread src/garage/envs/__init__.py Outdated
from garage.envs.half_cheetah_vel_env import HalfCheetahVelEnv
from garage.envs.normalized_env import normalize
from garage.envs.point_env import PointEnv
from garage.envs.rl2_env import RL2Env
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can this be part of the garage.tf.algos.rl2 module instead?

is there any way that RL2 can automatically wrap the environment with this wrapper?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes I will put this inside rl2 class and make RL2 automatically wrap the environment with this wrapper

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, the env never get passed to RL2 -- It only lives in the runner so RL2 can't wrap it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah. this will have to wait for the refactor where the algorithm owns the sampler.

can you please file an issue about this?

@ahtsan ahtsan merged commit 19cc3e4 into master Mar 3, 2020
@ahtsan ahtsan deleted the rl2env branch March 3, 2020 20:49
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.

3 participants