Fix crashing when no grasp pose found.#232
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a crash path when no grasp pose can be produced by making grasp-pose generation return an explicit success flag and providing a safe fallback pose in the tutorial.
Changes:
- Update
GraspGenerator.get_grasp_poses()to return(is_success, best_grasp_pose, best_open_length)and return a default pose when no valid grasp is found. - Update the grasp tutorial to handle failed grasp generation by logging a warning and falling back to the robot’s rest pose.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/tutorials/grasp/grasp_generator.py | Handles grasp-generation failures without crashing by falling back to a rest pose. |
| embodichain/toolkits/graspkit/pg_grasp/antipodal_generator.py | Adds a success flag + early returns when no valid grasp candidates exist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Returns: | ||
| A tuple ``(best_grasp_pose, best_open_length)`` where | ||
| ``best_grasp_pose`` is a ``(4, 4)`` homogeneous matrix and | ||
| ``best_open_length`` is a scalar. | ||
| is_success (bool): Whether a valid grasp pose is found. | ||
| best_grasp_pose (torch.Tensor): If a valid grasp pose is found, a tensor of shape (4, 4) representing the homogeneous transformation matrix of the best grasp pose in the world frame. Otherwise, an identity matrix. | ||
| best_open_length (float): If a valid grasp pose is found, a scalar representing the optimal gripper opening length. Otherwise, a zero tensor. | ||
|
|
There was a problem hiding this comment.
The return type/documentation for best_open_length is inconsistent with what the function actually returns. The type hint says float, the docstring mentions a "zero tensor" on failure, but the success path returns a 0-dim torch.Tensor (best_open_length) while failure returns a Python 0.0. Please make the return type consistent (either always return a Python float via .item() or update the type hint/docstring to torch.Tensor and return torch.tensor(0.0, device=...) on failure).
| Raises: | ||
| RuntimeError: If :meth:`generate` or :meth:`annotate` has not | ||
| been called yet. | ||
| """ | ||
| if self._hit_point_pairs is None: | ||
| raise RuntimeError( | ||
| logger.log_warning( | ||
| "No antipodal point pairs available. " | ||
| "Call generate() or annotate() first." | ||
| ) | ||
| return False, torch.eye(4, device=self.device), 0.0 |
There was a problem hiding this comment.
The docstring still claims this method raises RuntimeError when _hit_point_pairs is None, but the implementation now logs a warning and returns (False, I, 0.0) instead. Please update the Raises: section (or reintroduce the exception if this is intended to remain a programmer error).
| collision_threshold=0.0, | ||
| ) | ||
| if is_colliding.logical_not().sum() == 0: | ||
| logger.log_warning("No valid antipodal pairs after angle filtering.") |
There was a problem hiding this comment.
The warning message inside the collision-filtering failure path is incorrect/misleading: this branch triggers when all candidates collide ((~is_colliding).sum() == 0), but the log says "after angle filtering". Please change the message to reflect collision filtering (and ideally distinguish it from the earlier angle-filtering warning).
| logger.log_warning("No valid antipodal pairs after angle filtering.") | |
| logger.log_warning("No valid antipodal pairs after collision filtering.") |
| def get_grasp_poses( | ||
| self, | ||
| object_pose: torch.Tensor, | ||
| approach_direction: torch.Tensor, | ||
| visualize_collision: bool = False, | ||
| visualize_pose: bool = False, | ||
| ) -> tuple[torch.Tensor, torch.Tensor]: | ||
| ) -> tuple[bool, torch.Tensor, float]: |
There was a problem hiding this comment.
The class-level "Typical usage" snippet in this module still shows pose, open_length = generator.get_grasp_poses(...), but the method now returns (is_success, pose, open_length). Please update that example so users don't copy/paste a call that will raise a unpacking error.
Co-authored-by: chenjian <chenjian@dexforce.com>
Co-authored-by: chenjian <chenjian@dexforce.com>
Description
Fix crashing when no grasp pose found.
Type of change
Checklist
black .command to format the code base.