-
Notifications
You must be signed in to change notification settings - Fork 488
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
[habitat_31] Add Humanoid + ArticulatedAgent Tutorial tutorial #1765
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a first pass and left some comments. Looks good so far!
examples/tutorials/ao_tutorial.ipynb
Outdated
"id": "ba8f7681-d21a-417b-8370-7c684c2a8019", | ||
"metadata": {}, | ||
"source": [ | ||
"The first two sensors shown here are special. They are attached to a particular part of the agent, and will be updated as we udpate the agent. The third one is not attached to the agent.\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: udpate
examples/tutorials/ao_tutorial.ipynb
Outdated
"metadata": {}, | ||
"source": [ | ||
"## Defining new actions\n", | ||
"In the previous example we used actions to do navigation. We would like to also be able to pick up objects given an id. However, Habitat doesn't have a pre-defined action for that. We will look at a picking action here. You can find more information on adding actiions in TODO:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO remaining here.
habitat-lab/habitat/tasks/rearrange/actions/humanoid_actions.py
Outdated
Show resolved
Hide resolved
habitat-lab/habitat/tasks/rearrange/actions/humanoid_actions.py
Outdated
Show resolved
Hide resolved
vel = [0, turn_vel] | ||
return vel | ||
|
||
def lazy_inst_humanoid_controller(self, task, config): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we use self._task
instead of this parameter?
and angle_to_obj < self._config.turn_thresh | ||
) or dist_to_final_nav_targ < self._config.dist_thresh / 10.0 | ||
|
||
if self.motion_type == "base_velocity": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be cleaner to separate the base_velocity
and human_joints
cases in separate functions. Ideally, these should also be enums instead of strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it too, but it seems like we would need to pass a lot of arguments if we made them into seperate functions. Happy to hear your suggestions. I agree an enum would be better. Can we put an enum in a config though?
habitat-lab/habitat/tasks/rearrange/actions/oracle_nav_action.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made it through most of ao_tutorial notebook (through grasping).
Looks good overall, mostly nits.
I committed some changes to allow Jupyter notebook execution locally for me.
I'll add another review for the humanoids tutorial when I get a chance.
examples/tutorials/ao_tutorial.ipynb
Outdated
"\n", | ||
"object_trans = first_object.translation\n", | ||
"print(first_object.handle, \"is in\", object_trans)\n", | ||
"# TODO: unoccluded object did not work\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me like:
from habitat.datasets.rearrange.navmesh_utils import unoccluded_navmesh_snap
sample = unoccluded_navmesh_snap(
pos=object_trans,
height = 1.0,
pathfinder=sim.pathfinder,
sim=sim,
target_object_id=first_object.object_id,
)
Thank you both! FYI, I addressed all comments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once @xavierpuigf can validate everything is working rebased on main
we'll merge this. Any improvements can be iterative refactors. Thanks folks!
…ookresearch#1765) * Add tutorial * ao tutorial * updated tutorial * typos and private variables * rename snap pos * clarify param * modifications to run ao_tutorial in Jupyter notebook locally * addressing alex comments * download data paths : * remove todos * precommti --------- Co-authored-by: aclegg3 <alexanderwclegg@gmail.com>
…ookresearch#1765) * Add tutorial * ao tutorial * updated tutorial * typos and private variables * rename snap pos * clarify param * modifications to run ao_tutorial in Jupyter notebook locally * addressing alex comments * download data paths : * remove todos * precommti --------- Co-authored-by: aclegg3 <alexanderwclegg@gmail.com>
Motivation and Context
Tutorial to learn to controll humanoids in habitat
How Has This Been Tested
Ran notebook
Types of changes
Checklist