[BE] - cleanup ao humanoid controllers #1994
Conversation
facebook-github-bot
added
the
CLA Signed
habitat-lab/habitat/articulated_agent_controllers/humanoid_base_controller.py
Outdated
Show resolved
Hide resolved
habitat-lab/habitat/articulated_agent_controllers/humanoid_rearrange_controller.py
Outdated
Show resolved
Hide resolved
| """Set the speed of the humanoid according to the simulator speed""" | ||
| def set_framerate_for_linspeed( | ||
| self, lin_speed: float, ang_speed: float, ctrl_freq: int | ||
| ) -> None: |
There was a problem hiding this comment.
Unrelated to your changes:
The motion-driven approach to determine agent speed is problematic in some use cases as it caps the linear velocity defined in configs.
This had to be bypassed in HITL experiments to set humanoid speed. See this hack.
I would suggest that we tackle that in the following weeks so that we can have full functionality on main.
FYI @xavierpuigf
| @@ -107,7 +107,7 @@ def get_link_index(self, object_id: int) -> int: | |||
| ) | |||
| if ( | |||
| obj is not None | |||
| and isinstance(obj, ManagedBulletArticulatedObject) | |||
| and obj.is_articulated | |||
There was a problem hiding this comment.
Thanks :)
I think that, eventually, habitat-lab should provide a top-level class similar to this one to wrap the world state.
| # The last dimension contains the quaternion | ||
| return quat_tens / np.linalg.norm(quat_tens, axis=-1)[..., None] | ||
|
|
||
| def inter_data(x_i, y_i, z_i, dat, is_quat=False): | ||
| def inter_data( |
There was a problem hiding this comment.
I would rename this function to _trilinear_interpolation(...).
| @@ -601,10 +644,14 @@ def inter_data(x_i, y_i, z_i, dat, is_quat=False): | |||
| ) | |||
| return joint_list, transform | |||
|
|
|||
| def calculate_reach_pose(self, obj_pos: mn.Vector3, index_hand=0): | |||
| def calculate_reach_pose( | |||
There was a problem hiding this comment.
I would rename this to update_reach_pose(...) to convey that this is a stateful operation.
| value_norm_t = (index - lower) / -lower | ||
| lower = -1 | ||
|
|
||
| return lower, upper, value_norm_t | ||
|
|
||
| def comp_inter(x_i, y_i, z_i): | ||
| def comp_inter(x_i: int, y_i: int, z_i: int) -> int: |
There was a problem hiding this comment.
I would rename this function to _compute_pose_index(...). To be honest, I'm not sure what this really does though.
| num_bins: int, | ||
| value: float, | ||
| interp: bool = False, | ||
| ) -> Tuple[int, int, float]: |
There was a problem hiding this comment.
It would also be great to explain what this one does in more details.
|
@0mdc thanks for the review. Your comments on function names and design are valid. We're planning to refactor this code in upcoming BE, so I'll postpone those changes until we do so. |
Motivation and Context
This PR addresses two BE topics:
How Has This Been Tested
CI should test this
Note: until 2416 hits nightly this will fail.
Types of changes
Checklist