Conversation
dexscutai
commented
Feb 18, 2026
dexscutai
commented
There was a problem hiding this comment.
Pull request overview
This PR updates the semantic segmentation/mask labeling to split robot pixels into left vs right regions, enabling side-specific robot masking.
Changes:
- Extend
SemanticMaskenum from 3 classes (background/foreground/robot) to 4 classes (background/foreground/robot_left/robot_right). - Update
compute_semantic_mask()to compute separate masks for left and right robot links and include both in the stacked output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
embodichain/lab/gym/envs/managers/observations.py |
Computes separate left/right robot masks and returns a 4-channel semantic mask tensor. |
embodichain/data/enum.py |
Replaces ROBOT with ROBOT_LEFT and ROBOT_RIGHT in SemanticMask. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| left_robot_uids = torch.cat( | ||
| [ | ||
| env.robot.get_user_ids(link_name) | ||
| for link_name in env.robot.link_names | ||
| if link_name.startswith("left_") | ||
| ], | ||
| -1, | ||
| ) | ||
| right_robot_uids = torch.cat( | ||
| [ | ||
| env.robot.get_user_ids(link_name) | ||
| for link_name in env.robot.link_names | ||
| if link_name.startswith("right_") | ||
| ], | ||
| -1, | ||
| ) | ||
|
|
There was a problem hiding this comment.
torch.cat([...]) will raise at runtime if env.robot.link_names has no entries starting with left_ or right_ (empty list). To make this robust across robot models, consider getting all_robot_uids = env.robot.get_user_ids() once and slicing by the left/right link indices; if an index list is empty, use an empty tensor with the right shape/device so the mask simply becomes all-false instead of crashing.
| left_robot_uids = torch.cat( | |
| [ | |
| env.robot.get_user_ids(link_name) | |
| for link_name in env.robot.link_names | |
| if link_name.startswith("left_") | |
| ], | |
| -1, | |
| ) | |
| right_robot_uids = torch.cat( | |
| [ | |
| env.robot.get_user_ids(link_name) | |
| for link_name in env.robot.link_names | |
| if link_name.startswith("right_") | |
| ], | |
| -1, | |
| ) | |
| # Get all robot user IDs once and slice by left/right link indices. | |
| all_robot_uids = env.robot.get_user_ids() | |
| left_link_indices = [ | |
| i | |
| for i, link_name in enumerate(env.robot.link_names) | |
| if link_name.startswith("left_") | |
| ] | |
| right_link_indices = [ | |
| i | |
| for i, link_name in enumerate(env.robot.link_names) | |
| if link_name.startswith("right_") | |
| ] | |
| if left_link_indices: | |
| left_robot_uids = all_robot_uids[left_link_indices] | |
| else: | |
| left_robot_uids = all_robot_uids.new_empty((0,)) | |
| if right_link_indices: | |
| right_robot_uids = all_robot_uids[right_link_indices] | |
| else: | |
| right_robot_uids = all_robot_uids.new_empty((0,)) |
|
|
||
| masks = [None, None, None] | ||
| masks = [None, None, None, None] | ||
| masks_ids = [member.value for member in SemanticMask] | ||
| assert len(masks) == len( | ||
| masks_ids | ||
| ), "Different length of mask slots and SemanticMask Enum {}.".format(masks_ids) | ||
| mask_id_to_label = { | ||
| SemanticMask.BACKGROUND.value: background_mask, | ||
| SemanticMask.FOREGROUND.value: foreground_mask, | ||
| SemanticMask.ROBOT.value: robot_mask, | ||
| SemanticMask.ROBOT_LEFT.value: left_robot_mask, | ||
| SemanticMask.ROBOT_RIGHT.value: right_robot_mask, | ||
| } | ||
| for mask_id in masks_ids: | ||
| masks[mask_id] = mask_id_to_label[mask_id] |
There was a problem hiding this comment.
The semantic mask documentation and construction are now out of sync with the behavior: this code returns 4 channels (background/foreground/robot_left/robot_right), but the docstring above still describes 3 channels and the return shape as (num_envs, height, width). Also, hard-coding masks = [None, None, None, None] is brittle—prefer allocating based on len(SemanticMask) to avoid future enum changes silently breaking this function.
| BACKGROUND = 0 | ||
| FOREGROUND = 1 | ||
| ROBOT = 2 | ||
| ROBOT_LEFT = 2 | ||
| ROBOT_RIGHT = 3 |
There was a problem hiding this comment.
Renaming/removing SemanticMask.ROBOT is a breaking API change for any downstream code that imports that enum member or expects label value 2 to mean "robot" in general. If backward compatibility matters, consider keeping ROBOT = ROBOT_LEFT as an alias (or deprecating it) while introducing ROBOT_LEFT/ROBOT_RIGHT, and document how consumers should migrate.
1 participant
