Humanoids pr #1165

xavierpuigf · 2023-02-27T21:14:57Z

Motivation and Context

Adds humanoid control into Habitat. At the moment it adds a kinematically controlled humanoid, and a new action to control their joints and transform. This change is required to support more kinds of agents into Habitat.

How Has This Been Tested

Passes Pytests

Types of changes

Docs change / refactoring / dependency upgrade
New feature (non-breaking change which adds functionality)

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have completed my CLA (see CONTRIBUTING)
I have added tests to cover my changes.
All new and existing tests passed.

jimmytyyang · 2023-02-27T21:17:15Z

habitat-lab/habitat/articulated_agents/humanoids/kinematic_humanoid.py

+        return MobileManipulatorParams(
+            arm_joints=[],  # For now we do not add arm_joints
+            gripper_joints=[],
+            wheel_joints=None,


Thank you for working on this! Do we expect that humans have wheels?

No, but I found I could not ignore this parameter, even if it was optional. I think that in order to be able to ignore it, we need to set a default argument to wheels, here of None. I can do that.

Hm mobile_manipulator.py was probably designed with Fetch in mind, so I wouldn't change the default to None. Why not set it as False here instead of None?

We have here a check of wheel_joints existing as a param or being None, this is why I was proposing to set the default to None. If we set it as False, or [], we would need to change all these checks as well. I can do that if you prefer.

What is the type of this variable? This should indicate what the no wheel option should be.

Optional[List[int]]

yeah, I faced the same issue before, my solution at that time was to use inheritance, which is not the best way to do this

Could you elaborate on how you used inheritance? I think the best would be to make it List[int] (no optional) with a default argument [] and whenever we are checking this param, we replace the None/False checks fro empty list.

habitat-lab/habitat/articulated_agents/humanoids/kinematic_humanoid.py

akshararai

Looks good to me. Just needs some additional comments. Should we also add a humanoid URDF and some examples on how to do some random control with the humanoid?

habitat-lab/habitat/articulated_agents/README.md

habitat-lab/habitat/articulated_agents/articulated_agent_base.py

akshararai · 2023-02-27T23:19:04Z

habitat-lab/habitat/articulated_agents/humanoids/kinematic_humanoid.py

+        return MobileManipulatorParams(
+            arm_joints=[],  # For now we do not add arm_joints
+            gripper_joints=[],
+            wheel_joints=None,


Hm mobile_manipulator.py was probably designed with Fetch in mind, so I wouldn't change the default to None. Why not set it as False here instead of None?

habitat-lab/habitat/articulated_agents/humanoids/kinematic_humanoid.py

habitat-lab/habitat/articulated_agents/manipulator.py

habitat-lab/habitat/articulated_agents/mobile_manipulator.py

habitat-lab/habitat/tasks/rearrange/actions/actions.py

xavierpuigf · 2023-02-28T03:08:28Z

Looks good to me. Just needs some additional comments. Should we also add a humanoid URDF and some examples on how to do some random control with the humanoid?

Good point on the random control, I can add a functions in interactive_play that does that. Regarding the URDF, I made this PR to download the humanoid URDF in habitat-sim, @aclegg3, is that good to merge?

xavierpuigf · 2023-02-28T23:35:46Z

Looks good to me. Just needs some additional comments. Should we also add a humanoid URDF and some examples on how to do some random control with the humanoid?

Good point on the random control, I can add a functions in interactive_play that does that. Regarding the URDF, I made this PR to download the humanoid URDF in habitat-sim, @aclegg3, is that good to merge?

Alright, I added a random control demo in latest commit

habitat-lab/habitat/articulated_agents/articulated_agent_base.py

examples/interactive_play.py

vincentpierre · 2023-02-28T23:42:09Z

habitat-lab/habitat/articulated_agents/humanoids/kinematic_humanoid.py

+        return MobileManipulatorParams(
+            arm_joints=[],  # For now we do not add arm_joints
+            gripper_joints=[],
+            wheel_joints=None,


What is the type of this variable? This should indicate what the no wheel option should be.

habitat-lab/habitat/articulated_agents/humanoids/kinematic_humanoid.py

habitat-lab/habitat/tasks/rearrange/actions/actions.py

vincentpierre · 2023-02-28T23:52:38Z

habitat-lab/habitat/tasks/rearrange/actions/actions.py

+            self.cur_articulated_agent.set_joint_transform(
+                new_joints, new_transform
+            )
+        return self._sim.step(HabitatSimActions.changejoint_action)


I do not understand what this does. Why is this needed? What does self._sim need to update for the changejoint_action ?

I probably misunderstood the code, but don't we need to call sim.step on every step() function, since functions calling step expect observations from the environment? I think this makes sense when we call this function via the high level policy.

This could cause the simulator to step twice per high-level step. The action should only step the simulator if the action is the last action to execute. See how it's done in the other actions.

akshararai

Looks good to me. Once all comments are addressed, we can merge this soon I think. Thank you for working on this, and addressing all the comments!

examples/interactive_play.py

Co-authored-by: akshararai <akshara.rai@gmail.com>

akshararai

looks good to me.

* first commit general kinematic human model * correcting offset * update readme * linting * small fix and clarification updateds * fix import * linting * add download instructions for humanoid * pose getter * updated interactive play with humanoid * update readme * updated docstring * updated docstring * more docs and fix * improving interactive play * Update examples/interactive_play.py Co-authored-by: akshararai <akshara.rai@gmail.com> * improve name; * corrected typo --------- Co-authored-by: akshararai <akshara.rai@gmail.com>

xavierpuigf added 4 commits February 24, 2023 16:16

first commit general kinematic human model

2af43d3

correcting offset

371fa8a

update readme

4120e51

linting

0cf9e5f

xavierpuigf requested review from aclegg3, vincentpierre and jimmytyyang February 27, 2023 21:14

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 27, 2023

jimmytyyang reviewed Feb 27, 2023

View reviewed changes

akshararai reviewed Feb 28, 2023

View reviewed changes

xavierpuigf added 3 commits February 27, 2023 18:55

small fix and clarification updateds

524cbd0

fix import

5b80e42

linting

25cb19d

xavierpuigf added 4 commits February 28, 2023 08:51

add download instructions for humanoid

1618d74

pose getter

d87b616

updated interactive play with humanoid

f99142e

update readme

2ca5b88

xavierpuigf closed this Feb 28, 2023

xavierpuigf reopened this Feb 28, 2023

vincentpierre reviewed Feb 28, 2023

View reviewed changes

xavierpuigf added 3 commits February 28, 2023 16:04

updated docstring

2335380

updated docstring

1872561

more docs and fix

458ec00

akshararai reviewed Mar 1, 2023

View reviewed changes

xavierpuigf and others added 4 commits February 28, 2023 17:48

improving interactive play

a48ce9c

Update examples/interactive_play.py

69be0e9

Co-authored-by: akshararai <akshara.rai@gmail.com>

improve name;

2aa9261

corrected typo

554f1f5

akshararai approved these changes Mar 1, 2023

View reviewed changes

rpartsey self-requested a review March 1, 2023 07:55

xavierpuigf merged commit 0f811a4 into main Mar 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Humanoids pr #1165

Humanoids pr #1165

xavierpuigf commented Feb 27, 2023 •

edited

Loading

jimmytyyang Feb 27, 2023

xavierpuigf Feb 27, 2023

akshararai Feb 27, 2023

xavierpuigf Feb 28, 2023

vincentpierre Feb 28, 2023

xavierpuigf Feb 28, 2023

jimmytyyang Mar 1, 2023

xavierpuigf Mar 1, 2023

akshararai left a comment

akshararai Feb 27, 2023

xavierpuigf commented Feb 28, 2023

xavierpuigf commented Feb 28, 2023

vincentpierre Feb 28, 2023

vincentpierre Feb 28, 2023

xavierpuigf Mar 1, 2023

ASzot Mar 1, 2023

akshararai left a comment

akshararai left a comment

Humanoids pr #1165

Humanoids pr #1165

Conversation

xavierpuigf commented Feb 27, 2023 • edited Loading

Motivation and Context

How Has This Been Tested

Types of changes

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akshararai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xavierpuigf commented Feb 28, 2023

xavierpuigf commented Feb 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akshararai left a comment

Choose a reason for hiding this comment

akshararai left a comment

Choose a reason for hiding this comment

xavierpuigf commented Feb 27, 2023 •

edited

Loading