-
Notifications
You must be signed in to change notification settings - Fork 508
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
Fix controller #1202
Fix controller #1202
Conversation
examples/interactive_play.py
Outdated
pose, | ||
root_trans_offset, | ||
root_trans_base, | ||
) = humanoid_controller.get_walk_pose(relative_pos) | ||
base_action = humanoid_controller.vectorize_pose( | ||
pose, root_trans_offset, root_trans_base | ||
) |
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.
A tiny bit awkward that the 3 outputs of get_walk_pose are passed to vectorize_pose. Would it be simpler to have a humanoid_controller.action_from_relative_position that takes as input relative_pos
and returns base_action
(under the hood it would just call get_walk_pose then vectorize_pose.
If this is the only place this is used, maybe doing
full_pose = humanoid_controller.get_walk_pose(relative_pos)
base_action = humanoid_controller.vectorize_pose(* full_pose)
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.
Yes, I like that. I would like to keep the original functions that return the different components, in case we want to use part of the transform lately (for instance, compute the joint changes but not the base transform), but I can have a function that calls + vectorize under the hood.
habitat-lab/habitat/articulated_agent_controllers/humanoid_rearrange_controller.py
Outdated
Show resolved
Hide resolved
habitat-lab/habitat/articulated_agent_controllers/humanoid_rearrange_controller.py
Outdated
Show resolved
Hide resolved
habitat-lab/habitat/articulated_agent_controllers/humanoid_rearrange_controller.py
Outdated
Show resolved
Hide resolved
Thanks for the comments! I think everything is addressed |
…into fix_controller merge from main
Could you add some unit tests for the human controller and add end to end tests with the gym environments (create env, reset and do a few random actions on it) |
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.
Besides missing tests I think this looks very good.
Please add some tests before merging
* fix path human armature * update controller * fix * improving code for humanoid controller and moving to habitat-lab * renaming docstring * clean conversion degree_to_grads * improved controller * fix * adding tests humanoid
* fix path human armature * update controller * fix * improving code for humanoid controller and moving to habitat-lab * renaming docstring * clean conversion degree_to_grads * improved controller * fix * adding tests humanoid
Motivation and Context
This change improves the humanoid controller to support relative transforms without changing the base pose.
This allows to run path planner smoothly even when there are relative transforms caused by the walking gait.
It also moves the controller code from baselines into habitat-lab.
How Has This Been Tested
Passes tests
Ran Interactive Play Demo
Types of changes
Checklist