-
Notifications
You must be signed in to change notification settings - Fork 505
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
HITL - Synchronize UI with grasp manager. #1988
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.
Left a few questions, otherwise looks good.
I am assuming these are tested through phase3 deployment already, so logic is OK.
if TYPE_CHECKING: | ||
from habitat.tasks.rearrange.rearrange_grasp_manager import ( | ||
RearrangeGraspManager, | ||
) | ||
|
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.
OMG this is neat! I have run into so many circular imports for typing that I ended up removing in the past :o
for agent_index in range(len(agents_mgr.agent_names)): | ||
grasp_mgr = agents_mgr._all_agent_data[agent_index].grasp_mgr | ||
if grasp_mgr._snapped_obj_id == object_id: | ||
return True |
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.
This generally brings HitL + habitat-llm logic closer together ✔️
if self._automatically_update_snapped_object: | ||
self._snap_constraints = [ |
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.
Curious what is the use-case here?
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.
This is for dynamic simulations. The other code path is for kinematic simulations.
or not self._automatically_update_snapped_object | ||
): |
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.
Then is automatically updating == this code handling the update?
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.
Correct.
Because this class is the mechanism for communicating the grasping state of each agent, this short circuit allows for external code to handle the transform updates.
* Read grasp manager when querying whether an agent is carrying an object. * Communicate grasped objects to grasp manager.
* Read grasp manager when querying whether an agent is carrying an object. * Communicate grasped objects to grasp manager.
* Read grasp manager when querying whether an agent is carrying an object. * Communicate grasped objects to grasp manager.
Motivation and Context
The
RearrangeGraspManager
is used by agents to pick up objects or identify if an object is grabbed. Therefore, the objects grabbed inrearrange_v2
are not visible to the agents, while the objects grabbed by agents can be interacted by the UI.This changeset changes the
rearrange_v2
GUI such as the grabbed objects are synchronized with grasp managers.Note:
A hacky flag is introduced in
RearrangeGraspManager
,_automatically_update_snapped_object
, which toggles whether objects are automatically displaced. In HITL, we displace the objects ourselves to avoid occluding the view and communicate to the user that an object is grabbed (see #1908 for examples).Ideally, the "grasped object" data should be separated from the object displacement item. However, this is a significant refactor. Let me know if you have ideas in the thread.
How Has This Been Tested
Tested on single-learn and multi-player evaluation.
Types of changes
Checklist