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

[Feature] Return depth from RoboHiveEnv #2058

Merged
merged 6 commits into from
Apr 22, 2024

Conversation

sriramsk1999
Copy link
Contributor

Description

This PR adds a flag to enable returning of depth images from RoboHiveEnv.

Motivation and Context

Described in #2051
close #2051

  • I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

What types of changes does your code introduce? Remove all that do not apply:

  • New feature (non-breaking change which adds core functionality)
  • Documentation (update in the documentation)

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

  • I have read the CONTRIBUTION guide (required)
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

(I don't see any tests specific to RoboHiveEnv so I have not added any tests.)

Copy link

pytorch-bot bot commented Apr 6, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/2058

Note: Links to docs will display an error until the docs builds have been completed.

❌ 15 New Failures, 1 Unrelated Failure

As of commit 9c39412 with merge base 6b87184 (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

  • Examples Tests on Linux / tests (3.9, 12.1) / linux-job (gh)
    RuntimeError: This process waited for 1000.0 seconds without receiving a command from main. Consider increasing the maximum idle count if this is expected via the environment variable MAX_IDLE_COUNT (current value is 1000).

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@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 Apr 6, 2024
Copy link
Contributor

@vmoens vmoens left a comment

Choose a reason for hiding this comment

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

Cool! Can you also add a test in test_libs.py?

@sriramsk1999
Copy link
Contributor Author

While adding the test, I noticed that the current test code was actually broken. I've raised PR #2062 with the changes, but some tests are failing. Will rebase and add new tests after that is merged.

@vmoens vmoens added enhancement New feature or request Environments Adds or modifies an environment wrapper labels Apr 7, 2024
@vmoens
Copy link
Contributor

vmoens commented Apr 18, 2024

robohive tests raise an error bc the key can't be found, do you want me to investigate @sriramsk1999 ?

@sriramsk1999
Copy link
Contributor Author

I got probably the same error, only for the rpFrankaRobotiqData environment.

AssertionError: The keys of the specs and data do not match:
    - List of keys present in real but not in fake: set(),
    - List of keys present in fake but not in real: {('next', 'depths'), 'depths'}.

I haven't gotten around to checking out the error in detail yet, please feel free to have a look :) Will update if I find a fix.

@sriramsk1999
Copy link
Contributor Author

sriramsk1999 commented Apr 20, 2024

All RoboHive tests are now passing:

  • rpFrankaRobotiqData-v0 is the only environment that has pre-registered visual_keys, so from_pixels is overridden to True even if the user sets it to False. I don't think this is expected behaviour, so I've removed this snippet in this commit
  • I've added from_depths to the tests but there are now 2k tests just for RoboHive and it takes a while to complete (20 min on my PC)
  • Unit tests were failing because the environment was already registered. I've enabled overriding the already registered env here

Copy link
Contributor

@vmoens vmoens left a comment

Choose a reason for hiding this comment

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

LGTM

@vmoens vmoens merged commit 68ef60b into pytorch:main Apr 22, 2024
52 of 68 checks passed
@sriramsk1999 sriramsk1999 deleted the feature/robohive_depth branch April 22, 2024 15:14
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. enhancement New feature or request Environments Adds or modifies an environment wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants