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

Support MuJoCo 1.5 #767

Closed
wants to merge 3 commits into from
Closed

Support MuJoCo 1.5 #767

wants to merge 3 commits into from

Conversation

ethanabrooks
Copy link
Contributor

To test these changes, I checked that gym ran on all mujoco environments and that rendering works. Also, I have been working on a custom environment using these changes for about a month with no problems. That said, I cannot swear that these changes don't break anything so I certainly welcome any feedback.

@ethanabrooks ethanabrooks mentioned this pull request Nov 8, 2017
@@ -8,9 +8,9 @@ def __init__(self):
utils.EzPickle.__init__(self)

def _step(self, action):
xposbefore = self.model.data.qpos[0, 0]
xposbefore = self.sim.data.qpos[0]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that qpos lists joint angles. In half_cheetah.xml, it looks like the first joint is "rootx", a slide joint that should give displacement in the x direction. So qpos[0] should give the desired value.

vel_penalty = 1e-3 * v1**2 + 5e-3 * v2**2
alive_bonus = 10
r = (alive_bonus - dist_penalty - vel_penalty)[0]
r = alive_bonus - dist_penalty - vel_penalty
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure why alive_bonus - dist_penalty - vel_penalty used to be an array. Please double check if I broke something here.


def get_body_xmat(self, body_name):
idx = self.model.body_names.index(six.b(body_name))
return self.model.data.xmat[idx].reshape((3, 3))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These methods don't have obvious implementations in mujoco 1.5 and they aren't used anywhere so I deleted them.

@geyang
Copy link
Contributor

geyang commented Dec 14, 2017

@gdb would someone review this change? this one adds support for mujuco_py@1.5.
Thanks!

@aravindsrinivas
Copy link

@lobachevzky Does your GLFW in MuJoCo 1.5 close on calling env.close() ? I haven't tried using this branch but independently, the version I have has an issue with closing the renderer automatically from env.close() .... Is there some change to mujoco-py source that needs to be made?

@ethanabrooks
Copy link
Contributor Author

ethanabrooks commented Jan 18, 2018

@aravind0706 My pull request does not support closing on env.close(). The original version of mujoco-py calls env.render(close=True) -> self.get_viewer().finish() and the finish() method of MjViewer in mujoco-py does not exist in the latest version of mujoco-py.

What is your use case? I have never had to close a window programmatically (usually I use my good old mouse).

@matthiasplappert
Copy link
Contributor

I'm currently looking into getting this PR merged. I have benchmarked your changes with PPO to ensure that they are not broken and not much harder or easier to learn than before.

Here are these results:
fig_ant
fig_halfcheetah
fig_hopper
fig_humanoid
fig_humanoidstandup
fig_inverteddoublependulum
fig_invertedpendulum
fig_reacher
fig_swimmer
fig_walker2d

This looks very good overall. I'm just slightly concerned about Walker2d, but it seems to eventually converge to the same performance.

@matthiasplappert
Copy link
Contributor

Could you please bump the versions of all MuJoCo environments (e.g. HalfCheetah-v1 should become HalfCheetah-v2 and so on)? After this, I think we can get this merged quickly.

@matthiasplappert
Copy link
Contributor

I've moved your commits over to #834 and made the necessary version bumps myself. Closing this since further progress will be tracked in the new PR. Thank you so much for this contribution!

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.

4 participants