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 pybullet-based Gym Environments #87

Merged
merged 4 commits into from Dec 2, 2021

Conversation

gauravmm
Copy link
Contributor

@gauravmm gauravmm commented Jun 4, 2021

Don't accept this yet -- this is still a work-in-progress. Remaining work:

General-purpose environment loader:

  • Agree on interface
  • Refactor mujoco.py

Add support for freezing environments:

  • Locomotors
  • Manipulators
  • Pendula

Add documentation for:

  • Installing/using PyBullet
  • Various functions in mujoco.py
  • Comparing RobotSchool and MuJoCo-compatible PyBullet environments.

Tests:

  • Freezing environments.
  • Comparison between MuJoCo-compatible PyBullet and actual MuJoCo environments.

Other:

  • Gracefully handle case that PyBullet is not installed.
  • Properly package pybullet-gym
    • setup.py needs to copy 3d assets as well.
    • (Optional) Put it on Pip

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Motivation and Context / Related issue

This adds support for PyBullet, an open-source alternative to MuJoCo. MuJoCo-compatible and RobotSchool environments are supported via pybullet-gym.

How Has This Been Tested (if it applies)

Using this for research.

Checklist

  • The documentation is up-to-date with the changes I made.
  • I have read the CONTRIBUTING document and completed the CLA (see CONTRIBUTING).
  • All tests passed, and additional code has been covered with new tests.

@facebook-github-bot
Copy link

Hi @gauravmm!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@a3ahmadfb
Copy link

a3ahmadfb commented Jun 4, 2021

Should there be a separate mbrl/util/pybullet.py instead of adding pybullet envs to mujoco.py? Or, perhaps, a general interface that's not (explicitly or implicitly) tied to a specific engine?

Looks good, though, glad you're doing this!

@luisenp
Copy link
Contributor

luisenp commented Jun 4, 2021

Should there be a separate mbrl/util/pybullet.py instead of adding pybullet envs to mujoco.py? Or, perhaps, a general interface that's not (explicitly or implicitly) tied to a specific engine?

Thanks for the contribution @gauravmm, this is great! The quote above was going to be my first suggestion. But, in general, do you want me to start reviewing the code as it is, or want to finish other changes first? (asking given your first sentence in the description that says code not ready yet).

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 4, 2021
@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@gauravmm
Copy link
Contributor Author

gauravmm commented Jun 4, 2021

Let me finish the rest of the code.

It seems to me like the existing mujoco.py supports loading both dmcontrol and mujoco environments; I like @a3ahmadfb's idea to use a common interface -- I could work on that in a week or two.

@gauravmm gauravmm marked this pull request as draft June 4, 2021 21:33
@luisenp
Copy link
Contributor

luisenp commented Jun 7, 2021

Let me finish the rest of the code.

It seems to me like the existing mujoco.py supports loading both dmcontrol and mujoco environments; I like @a3ahmadfb's idea to use a common interface -- I could work on that in a week or two.

Sounds good, I'll wait. Regarding mujoco.py, note that dmcontrol also uses Mujoco, which is why they are both included in the same module. The rationale for this file was to isolate Mujoco imports from the rest of the library, since it requires a special library and I didn't want to add overly restrictive dependencies to the core components. For this PR, I suggest to start with a separate file for pybullet, trying to keep a common interface if possible, but no need to go out of your way to make it general.

@luisenp
Copy link
Contributor

luisenp commented Jul 7, 2021

Hi @gauravmm, just checking if you had any questions about this or generally checking if there is anything I can do to help.

@gauravmm
Copy link
Contributor Author

Oh, oops. I haven't had the time to look into this further.

To check the initial code (before the planned refactor), I ran the PyBullet "MuJoCo-clone" environment and I could not get your vanilla MBPO to learn well. This is most likely because there's some part of the state that I am not freezing correctly, or there's some issue with running multiple PyBullet instances in parallel.

I'm in the thick of working on a paper right now, and I'll get back to this when I have some free time. Until then, please feel free to delegate this to someone else or see if you can spot the issue.

@luisenp
Copy link
Contributor

luisenp commented Jul 12, 2021

Hi @gauravmm thanks for the update, and no worries. To clarify, do you perhaps mean CEM controller instead of MBPO? So far, I've use the freeze functionality mostly in the diagnostics to test the trajectory optimizer and the visualization videos, but not in MBPO.

Incidentally, I happened to start looking at MBPO on PyBullet, as another student reported issues with learning and I'm trying to debug learning in the Hopper and HalfCheetah versions, where the SAC agent diverges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants