Skip to content

Fix semantic mask robot user_ids indexing#222

Merged
yuecideng merged 2 commits into
mainfrom
fix/semantic-mask-user-ids-indexing
Apr 7, 2026
Merged

Fix semantic mask robot user_ids indexing#222
yuecideng merged 2 commits into
mainfrom
fix/semantic-mask-user-ids-indexing

Conversation

@yuecideng
Copy link
Copy Markdown
Contributor

Description

This PR fixes the compute_semantic_mask function in the observation manager. The previous implementation called get_user_ids(link_name) per link in a list comprehension and concatenated the results, which produced incorrect user ID gathering. The fix builds left/right link index lists first via enumerate, then indexes robot.user_ids[:, link_indices] in a single tensor operation.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which improves an existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (existing functionality will not work without user modification)
  • Documentation update

Checklist

  • I have run the black . command to format the code base.
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • Dependencies have been updated, if applicable.

🤖 Generated with Claude Code

Build left/right link index lists first, then index robot.user_ids
in a single tensor operation instead of calling get_user_ids per link
and concatenating. This fixes incorrect uid gathering and is more
efficient.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 7, 2026 05:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes incorrect per-environment robot user-ID aggregation in compute_semantic_mask, ensuring left/right robot link user IDs are gathered and compared against the segmentation mask with the correct tensor shape semantics.

Changes:

  • Replaces per-link get_user_ids(link_name) concatenation (which flattened across envs) with per-link index collection via enumerate.
  • Indexes env.robot.user_ids[:, link_indices] to keep user IDs in (num_envs, num_links_side) form for correct broadcasting/masking.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +361 to +366
left_link_indices = [
i
for i, link_name in enumerate(env.robot.link_names)
if link_name.startswith("left_")
]
left_robot_uids = env.robot.user_ids[:, left_link_indices]
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated left/right user ID indexing fixes a per-environment shape issue, but there is no unit test covering compute_semantic_mask. Please add a test (in tests/gym/envs/managers/test_observation_functors.py, where other observation functors are tested) that uses >1 env with distinct robot.user_ids per env and verifies the resulting left/right robot mask channels are computed per-env (not flattened/concatenated across envs).

Copilot uses AI. Check for mistakes.
@yuecideng yuecideng merged commit 50c8119 into main Apr 7, 2026
10 of 12 checks passed
@yuecideng yuecideng deleted the fix/semantic-mask-user-ids-indexing branch April 7, 2026 07:15
yuecideng added a commit that referenced this pull request Apr 10, 2026
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
chase6305 pushed a commit that referenced this pull request Apr 16, 2026
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants