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

Refactor Mujoco Rendering and bump to Mujoco 2.3.0 #112

Conversation

rodrigodelazcano
Copy link
Member

@rodrigodelazcano rodrigodelazcano commented Nov 3, 2022

Description

This PR refactors the mujoco rendering:

  • Created a new class MujocoRenderer. This class abstracts the viewer (offscreen or window) selection depending on the render_mode.
  • Solved issue from the gym repository. Only one glfw context can be made current in the main thread. The Offscreen and Window renders switch their context current status. Also note that this only works if the Offscreen renderer uses glfw as the opengl backend, if the environment variable is MUJOCO_GL=glfw, not for egl or osmesa which are usually used for headless rendering (no "human" rendering permitted either way).

New additions:

  • Bump mujoco to version 2.3.0

Please delete options that are not relevant.

  • 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 not work as expected)
  • This change requires a documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@rodrigodelazcano
Copy link
Member Author

Hey @tudorjnu. Will this PR fix your issue in openai/gym#2989? I've used it to render an rgb_array camera sensor at the same time I debug with human rendering in another camera.

Copy link
Member

@pseudo-rnd-thoughts pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

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

As this PR solves the issue that you note

Im uncertain on removing viewer_setup as this prevents generic environment implementations however I think this is not a great issue.

Otherwise I can spot minimal issues

self.viewer = None
self._viewers = {}

def viewer_setup(self):
Copy link
Member

Choose a reason for hiding this comment

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

Why does this function still exist if we are using the default_camera_config parameter

Copy link
Member Author

Choose a reason for hiding this comment

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

This function only concerns the mujoco environments that depend on mujoco_py which are not maintained.

def __del__(self):
self.free()

def render(self):
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment explaining this function

@rodrigodelazcano rodrigodelazcano changed the title Refactor Mujoco Rendering Refactor Mujoco Rendering and bump to Mujoco 2.3.0 Dec 1, 2022
Copy link
Member

@pseudo-rnd-thoughts pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

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

This is a complex PR that I can't follow all the way through but if you are happy with it, Im happy to merge

@tudorjnu
Copy link

Hello @rodrigodelazcano! Thank you for the PR. Unfortunately, it seems like it did not fix the problem. Firstly, it seems that the renderer can render a single camera at a time given the current implementation. I manage to get away with it by redefining the render method and allowing myself to add the parameters:

    def render(
            self,
            render_mode: str = None,
            camera_id: int = None,
            camera_name: str = None
    ):
        if render_mode is None:
            render_mode = self.render_mode
        if camera_id is None:
            camera_id = self.camera_id
        if camera_name is None:
            camera_name = self.camera_name

        return self.mujoco_renderer.render(
            render_mode, camera_id, camera_name
        )

Still, rendering the "rgb_array" and "human" at the same time is not possible at this moment. However, I think that allowing the user to change the rendering values as above offers more versatility. Whilst it does not work with the human mode, at least it can generate multiple RGB images from the simulator. This is useful if, for example, someone would want to get images from different angles and form an observation space in this way or get the image and the segmentation mask.

@rodrigodelazcano
Copy link
Member Author

Hey @tudorjnu, thank you for replying back. What is your use case? The purpose of the render() method is to not have any arguments pass and only use the paramaters set at initialization (render_mode, camera_id, ...). This method is only used for visualization, not for observation generation.

However, if you want to add images to your observations from other cameras you can still do that. You can call self.mujoco_renderer.render("rgb_array", camera_id, camera_name) while generating your observation. This should work even if initializing the environment with render_mode="human".

Also, are you using the MujocoEnv class for your custom environment?

@tudorjnu
Copy link

Hello @rodrigodelazcano! Thanks for the prompt answer. Indeed, it seems to work in the way you mentioned. Massive thanks for that! 😁

I thought I tried to render an image that way but it seems I did not! Cheers for your help!

Also, what would be the easiest way to access individual camera information (e.g. extracting segmentation masks from the camera)?

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