Skip to content

Conversation

@bikcrum
Copy link
Contributor

@bikcrum bikcrum commented Dec 7, 2025

Description

Bug: FrameTransformer uses only the body name (last component of the prim path) as the unique key to identify bodies. This causes target frames with the same body name but different hierarchical paths to collide, resulting in only one body being tracked.

Example: When tracking both Robot/left_hand and Robot_1/left_hand:

  • Both resolve to body_name = "left_hand"
  • Only the first one registered gets tracked
  • Both left_hand and left_hand_1 target frames return identical (incorrect) transforms

Use cases affected:

  • Multi-robot scenarios (e.g., Robot/left_hand vs Robot_1/left_hand)
  • Single articulation with duplicate body names at different hierarchy levels (e.g., arm/link vs leg/link)

Fix: Use the relative prim path (path after env_X/) as the unique body identifier instead of just the body name. For example:

  • /World/envs/env_0/Robot/left_hand -> key: Robot/left_hand
  • /World/envs/env_0/Robot_1/left_hand -> key: Robot_1/left_hand

Fixes #4167

Note: While users could work around this by creating separate FrameTransformer instances, this is not an acceptable solution because:

  1. The current behavior silently returns incorrect data without any warning
  2. The API contract (specifying explicit prim_paths) implies distinct bodies should be tracked
  3. The workaround adds unnecessary complexity and memory overhead

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

N/A - Internal data structure fix. No visual changes.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@bikcrum bikcrum requested a review from jtigue-bdai as a code owner December 7, 2025 23:36
@github-actions github-actions bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Dec 7, 2025
@bikcrum bikcrum changed the title Bugfix/frame transformer body name [Fix] FrameTransformer body name collision for duplicate names at different hierarchy levels Dec 7, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 7, 2025

Greptile Overview

Greptile Summary

Fixes critical bug where FrameTransformer incorrectly tracked bodies with identical leaf names but different hierarchical paths (e.g., Robot/left_hand vs Robot_1/left_hand). The fix changes the unique body identifier from the leaf name to the relative prim path after env_X/.

Key changes:

  • Uses regex to extract relative path (Robot/left_hand instead of just left_hand) as the unique identifier in body_names_to_frames dictionary
  • Updates _target_frame_body_names list generation to use relative paths
  • Handles source frame extraction from config patterns containing {ENV_REGEX_NS} or env_X/
  • Adds comprehensive test with two robots (2m apart) tracking same-named bodies, verifying distinct transforms

Impact:

  • Multi-robot scenarios now work correctly
  • No breaking changes to existing single-robot functionality
  • Proper version bump (patch: 0.49.0 → 0.49.1) and changelog entry

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - it fixes a genuine bug with a well-tested solution
  • Score reflects solid implementation with comprehensive test coverage. The fix correctly addresses the root cause by using relative prim paths instead of leaf names. The regex patterns handle the expected path formats. Minor deduction for regex edge cases: the pattern env_\d+/(.*) may not match paths without environment prefixes in non-standard setups, though fallback to leaf name should handle this
  • No files require special attention - the changes are well-contained and properly tested

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab/isaaclab/sensors/frame_transformer/frame_transformer.py 4/5 Changes body identifier from leaf name to relative prim path, fixing collision bug when tracking same-named bodies from different hierarchies
source/isaaclab/test/sensors/test_frame_transformer.py 5/5 Adds comprehensive test with two robots tracking same-named bodies, verifies distinct transforms are returned

Sequence Diagram

sequenceDiagram
    participant User
    participant FrameTransformerCfg
    participant FrameTransformer
    participant PhysicsView
    participant BodyDict as body_names_to_frames Dict

    User->>FrameTransformerCfg: Configure target frames<br/>(Robot/LF_SHANK, Robot_1/LF_SHANK)
    
    User->>FrameTransformer: Initialize sensor
    
    Note over FrameTransformer: _initialize_impl()
    
    FrameTransformer->>FrameTransformer: Iterate through target frames
    
    FrameTransformer->>FrameTransformer: Resolve prim path<br/>/World/envs/env_0/Robot/LF_SHANK
    
    FrameTransformer->>FrameTransformer: Extract relative path using regex<br/>env_\d+/(.*)
    
    Note over FrameTransformer: OLD: body_name = "LF_SHANK"<br/>NEW: body_name = "Robot/LF_SHANK"
    
    FrameTransformer->>BodyDict: Store body_name → {frames, prim_path, type}
    
    FrameTransformer->>FrameTransformer: Resolve prim path<br/>/World/envs/env_0/Robot_1/LF_SHANK
    
    FrameTransformer->>FrameTransformer: Extract relative path<br/>env_\d+/(.*)
    
    Note over FrameTransformer: OLD: body_name = "LF_SHANK" (COLLISION!)<br/>NEW: body_name = "Robot_1/LF_SHANK" (UNIQUE)
    
    FrameTransformer->>BodyDict: Store body_name → {frames, prim_path, type}
    
    Note over BodyDict: OLD: Only 1 entry (collision)<br/>NEW: 2 separate entries
    
    FrameTransformer->>FrameTransformer: Build target frame body names list<br/>using _get_rel_path()
    
    FrameTransformer->>PhysicsView: Create view with tracked_body_names<br/>["Robot/LF_SHANK", "Robot_1/LF_SHANK"]
    
    Note over PhysicsView: Both bodies tracked separately
    
    FrameTransformer->>User: Returns distinct transforms<br/>for each body
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@bikcrum bikcrum changed the title [Fix] FrameTransformer body name collision for duplicate names at different hierarchy levels Fixes FrameTransformer to handle same body names at different hierarchy levels Dec 8, 2025
@kellyguo11 kellyguo11 moved this to In review in Isaac Lab Dec 10, 2025
@bikcrum bikcrum force-pushed the bugfix/frame-transformer-body-name branch 3 times, most recently from 231526e to 03def46 Compare December 13, 2025 21:04
Copy link
Collaborator

@jtigue-bdai jtigue-bdai left a comment

Choose a reason for hiding this comment

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

Hey thanks for the feature. Some minor stuff with the test.

@bikcrum bikcrum force-pushed the bugfix/frame-transformer-body-name branch from 2266636 to b382b4d Compare December 18, 2025 06:52
Copy link
Collaborator

@jtigue-bdai jtigue-bdai left a comment

Choose a reason for hiding this comment

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

NIT in the test, but thanks for the added functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

[Bug Report] FrameTransformer returns identical transforms for bodies with same name but different paths

2 participants