[WIP] - Embodied Unoccluded Navmesh Snap #1778
Conversation
facebook-github-bot
added
the
CLA Signed
| self.cur_articulated_agent, | ||
| start_pos, _, _ = embodied_unoccluded_navmesh_snap( | ||
| target_position=np.array(obj_pos), | ||
| height=1.5, # NOTE: this is default agent max height. This parameter is used to determine whether or not a point is occluded. |
There was a problem hiding this comment.
Why is this hardcoded? Shouldn't this be retrieved from the agent config?
| search_offset=self._config.spawn_max_dist_to_obj, | ||
| max_samples=self._config.num_spawn_attempts, | ||
| agent_embodiement=self.cur_articulated_agent, | ||
| target_object_id=None, # TODO: this must be the integer id of the target object or no unoccluded state will be found because this object will be considered occluding |
There was a problem hiding this comment.
Why set to None here? Shouldn't this be set to the target entity we are navigating to? Or is the current code just the beginning and we will address this in a later PR?
| @@ -419,6 +419,7 @@ def set_articulated_agent_base_to_random_point( | |||
| island_index=self._largest_indoor_island_idx | |||
| ) | |||
|
|
|||
| # TODO: is this correct behavior? | |||
| start_pos = self.safe_snap_point(start_pos) | |||
There was a problem hiding this comment.
Yeah, I think this is correct. This is just to double check the position is a valid point on the navmesh. Maybe this call isn't required, but I don't want to risk removing it.
| When that point returns NaN, computes a navigable point at increasing | ||
| distances to it. | ||
| """ | ||
| # TODO: is this correct behavior? |
There was a problem hiding this comment.
What is your concern about the logic here?
| @@ -785,3 +606,197 @@ def get_camera_lookat_relative_to_vertial_line( | |||
| # Get angle between location and the vector | |||
| angle = get_camera_object_angle(cam_T, vertical_dir, local_vertical_dir) | |||
| return angle | |||
|
|
|||
|
|
|||
| def embodied_unoccluded_navmesh_snap( | |||
There was a problem hiding this comment.
My primary concern about this change is the behavior is fundamentally different from the previous _get_robot_spawns. If robot spawns failed, it would still return some starting configuration for the robot. However, if this fails, it will return None, and presumably the robot will not be moved. I think it's dangerous not to have this fall back of placing the robot at least somewhere reasonable, even if it's in collision. Not moving the robot at all can cause big problems in reward calculation that are hard to catch.
| search_offset=self._config.spawn_max_dist_to_obj, | ||
| max_samples=self._config.num_spawn_attempts, | ||
| agent_embodiement=self.cur_articulated_agent, | ||
| target_object_id=None, # TODO: this must be the integer id of the target object or no unoccluded state will be found because this object will be considered occluding |
There was a problem hiding this comment.
could this be a receptacle ID? A task where the robot wants to place the object on the receptacle
| test_batch: List[Tuple[mn.Vector3, float]] = [] | ||
| sample_count = 0 | ||
| while len(test_batch) < test_batch_size and sample_count < max_samples: | ||
| sample = pathfinder.get_random_navigable_point_near( |
There was a problem hiding this comment.
Do we know that this sample always returns a valid point? In other words, we do not want to have NaN cases
| start_position = agent_embodiement.base_pos | ||
| start_rotation = agent_embodiement.base_rot | ||
|
|
||
| agent_embodiement.base_pos = batch_sample[0] |
There was a problem hiding this comment.
There is a case that once agent_embodiement.base_pos is assigned to nan point, we cannot reset the agent pose by giving a valid point. do we want to add a check for not assigning nan point?
| return batch_sample[0], desired_angle, True | ||
|
|
||
| # unable to find a valid navmesh point within constraints | ||
| return None, None, False |
There was a problem hiding this comment.
at least we can return something to set the agent pose after best effect of searching?
Motivation and Context
TL;DR: replace varied articulated_agent state sampling functionality with one unified interface.
Uses the unoccluded snapping function as a base and adds embodiment collision checking options.
How Has This Been Tested
Locally with a viewer application.
TODO: should test the downstream users to verify that new configuration and use-cases are fully covered. This is not done for the current state, so I am fairly confident the current useage is faulty already.
Example of heuristic embodiment snapping with examples/rearrange_ep_viewer.py demo app:
embodied_heuristic_snap-2024-01-29_13.04.43.mp4
Types of changes
Checklist