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

envs robotics - touch sensors - Shadow Hand #1299

Merged
merged 15 commits into from
Feb 9, 2019

Conversation

ndrwmlnk
Copy link
Contributor

@ndrwmlnk ndrwmlnk commented Jan 28, 2019

@matthiasplappert
We (@ndrwmlnk @llach) covered the hand model with 92 touch sensors and extended the HandManipulate{Block, Egg, Pen}-v0 environments to HandManipulate{Block, Egg, Pen}TouchSensors-v0. Now, an observation additionally contains 92 touch values (0 - no touch, 1 - touch detected). Sensory information allows faster and better convergence (preliminary results).

Test the environments with pre-trained weights (currently in training):
https://rebrand.ly/TouchSensors
python baselines/her/experiment/play.py HandManipulateBlockTouchSensors.pkl
python baselines/her/experiment/play.py HandManipulateEggTouchSensors.pkl
python baselines/her/experiment/play.py HandManipulatePenTouchSensors.pkl

Touch sensor sites are tailored to physical geoms (robot0:DC_Hand), thus may visually overlap with meshes (robot0:D_Vizual). Active touch sensors are highlighted in red, inactive sensors - in green:
https://rebrand.ly/TouchSensors
sensors_cube_geoms.mp4
sensors_cube_mesh.mp4
sensors_cube.mp4
sensors.mp4

@matthiasplappert
Copy link
Contributor

Thanks for your PR! I'm excited to get this merged and will work with @pzhokhov to figure out the best next steps.

Copy link
Contributor

@matthiasplappert matthiasplappert left a comment

Choose a reason for hiding this comment

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

Thanks again for this PR! I'm excited to get it merged soon.

I did an initial review and left some comments. Please address them and then I'll take another look. This PR will automatically update when you push to your branch, so just do that and let me know once ready.

gym/envs/robotics/assets/hand/shared.xml Outdated Show resolved Hide resolved
gym/envs/robotics/hand/manipulate_touch_sensors.py Outdated Show resolved Hide resolved
gym/envs/__init__.py Show resolved Hide resolved
gym/envs/robotics/hand/manipulate_touch_sensors.py Outdated Show resolved Hide resolved
gym/envs/robotics/hand/manipulate_touch_sensors.py Outdated Show resolved Hide resolved
gym/envs/robotics/hand/manipulate_touch_sensors.py Outdated Show resolved Hide resolved
gym/envs/robotics/hand/manipulate_touch_sensors.py Outdated Show resolved Hide resolved
gym/envs/robotics/hand/manipulate_touch_sensors.py Outdated Show resolved Hide resolved
gym/envs/robotics/hand/manipulate_touch_sensors.py Outdated Show resolved Hide resolved
@pzhokhov
Copy link
Collaborator

pzhokhov commented Feb 1, 2019

my 2 cents here are the following - a couple of tests of the new environments are failing (tried running manually, unfortunately, those won't show in travis because this is an external PR and as such our mujoco key is not injected => no tests of mujoco-based envs are run). Please run them manually (via pytest . with MUJOCO_KEY env variable set up and make sure 1) tests pass and 2) test count is 359 (that's how much it counted for me when I tried cloning the PR branch and running tests there). If there are some failures that seem unrelated to your changes, let's work together and figure out what's wrong

ndrwmlnk and others added 2 commits February 1, 2019 06:43
…s_85.xml; ManipulateTouchSensorsEnv._render_callback()

Co-authored-by: Luca Lach <[email protected]>
@ndrwmlnk
Copy link
Contributor Author

ndrwmlnk commented Feb 4, 2019

@matthiasplappert @pzhokhov
Thanks for reviewing the code and tips with pytest .

I improved the code and fixed requested changes. 376 tests passed without errors. The number of tests exceeds 359, probably because of the new touch-sensory robotic environments.
The reason for the previous pytest error was the length of the observation vector:

assert ob_space.contains(ob), 'Reset observation: {!r} not in space'.format(ob)

I added obs to ManipulateTouchSensorsEnv.init here:
https://github.com/ndrwmlnk/gym/blob/0d71b27103cd434173eb24ca50a99b3968866cfd/gym/envs/robotics/hand/manipulate_touch_sensors.py#L59
which now overrides obs from RobotEnv.init here (without touch values):

obs = self._get_obs()

Please let me know if I can make any further improvements.
Check pre-trained weights and videos here: https://rebrand.ly/TouchSensors

@pzhokhov
Copy link
Collaborator

pzhokhov commented Feb 6, 2019

@matthiasplappert @ndrwmlnk confirmed - tests are passing; if @matthiasplappert is happy, I am happy :)

@pzhokhov
Copy link
Collaborator

pzhokhov commented Feb 6, 2019

here's a successful travis build with the changes: https://travis-ci.org/openai/gym/jobs/489766042 (pay the "All checks have failed" no heed, I messed something up with github webhook when trying to set a build with the changes up)

@matthiasplappert
Copy link
Contributor

Will take another pass tomorrow (crazy busy week so far) and then we can hopefully get this merged. Thanks everyone for helping!

Copy link
Contributor

@matthiasplappert matthiasplappert left a comment

Choose a reason for hiding this comment

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

LGTM now. Also verified that this works properly when building for a PyPI release and that all necessary assets are included. Merging this now.

@matthiasplappert matthiasplappert merged commit 332732e into openai:master Feb 9, 2019
@matthiasplappert
Copy link
Contributor

And thanks for working with us on this PR; happy that we got it merged :)

kiku-jw pushed a commit to kiku-jw/gym that referenced this pull request Feb 23, 2019
* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

Co-authored-by: Luca Lach <[email protected]>

* add touch sensors layout

* add touch sensors layout

* extending robotics shadow hand with touch sensors

Co-authored-by: Luca Lach <[email protected]>

* extending robotics shadow hand with touch sensors

* add touch sensors layout

Co-authored-by: Luca Lach <[email protected]>

* update touch sensors layout 85

Co-authored-by: Luca Lach <[email protected]>

* add manipulate_egg_touch_sensors_85.xml & manipulate_pen_touch_sensors_85.xml; ManipulateTouchSensorsEnv._render_callback()

Co-authored-by: Luca Lach <[email protected]>

* add shared_touch_sensors_92.xml; solve pytest . failure

Co-authored-by: Luca Lach <[email protected]>
zlig pushed a commit to zlig/gym that referenced this pull request Apr 24, 2020
* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

* extending robotics shadow hand with touch sensors

Co-authored-by: Luca Lach <[email protected]>

* add touch sensors layout

* add touch sensors layout

* extending robotics shadow hand with touch sensors

Co-authored-by: Luca Lach <[email protected]>

* extending robotics shadow hand with touch sensors

* add touch sensors layout

Co-authored-by: Luca Lach <[email protected]>

* update touch sensors layout 85

Co-authored-by: Luca Lach <[email protected]>

* add manipulate_egg_touch_sensors_85.xml & manipulate_pen_touch_sensors_85.xml; ManipulateTouchSensorsEnv._render_callback()

Co-authored-by: Luca Lach <[email protected]>

* add shared_touch_sensors_92.xml; solve pytest . failure

Co-authored-by: Luca Lach <[email protected]>
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