Skip to content

Conversation

@AntoineRichard
Copy link
Collaborator

Description

Fixes a bug in the mask creation when only a partial list of joints / body were passed.

Fixes #4439

Type of change

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

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

… at least). There are still some errors some will be patched by latest newton.
…lean-up session with a Newton Eng to sanity check this.
…position as this is now handled properly by newton. Still a couple of weird cases left.
…rovided

Fixes GitHub issue isaac-sim#4439.

When joint_subset or body_subset is provided, the returned indices were
relative to the subset (local indices) instead of the full joint_names
or body_names list (global indices). This caused incorrect mask values.

The fix maps matched names back to their global indices when a subset
is explicitly provided, with zero overhead for the default case.

Also adds body_subset parameter to find_bodies for consistency.
@github-actions github-actions bot added the bug Something isn't working label Jan 26, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 26, 2026

Greptile Overview

Greptile Summary

Fixed critical indexing bug in find_joints and find_bodies functions when using subset parameters. The bug caused local indices (relative to the subset) to be returned instead of global indices (relative to the full list), resulting in incorrect mask values.

Core Fix:

  • Modified find_joints and find_bodies in source/isaaclab_newton/isaaclab_newton/assets/utils/shared.py to map matched names back to global indices when a subset is provided
  • Added body_subset parameter to find_bodies for consistency with find_joints
  • Zero overhead when subset is not used (backwards compatible)

Additional Changes:
This PR also includes substantial refactoring and new features beyond the stated bug fix:

  • New WrenchComposer utility class for force/torque composition
  • Extensive test infrastructure additions (mock utilities, benchmarks, integration tests)
  • Refactoring of RigidObjectData with new properties (is_primed, body_mass, body_inertia)
  • Various improvements to articulation and rigid object implementations

Test Coverage:

Confidence Score: 4/5

  • This PR is safe to merge with minor review recommended for scope beyond the stated bug fix
  • The core bug fix in shared.py is clean, well-tested, and directly addresses issue [Bug Report] find_joints in isaaclab_newton returns incorrect mask/indices when joint_subset is provided #4439. However, the PR bundles significant additional work (new WrenchComposer, extensive refactoring, test infrastructure) that wasn't mentioned in the PR description. The fix itself is solid with excellent test coverage, but the large scope increases merge risk.
  • Pay attention to source/isaaclab_newton/isaaclab_newton/assets/rigid_object/rigid_object_data.py which has extensive refactoring including new properties and state management changes that warrant careful review

Important Files Changed

Filename Overview
source/isaaclab_newton/isaaclab_newton/assets/utils/shared.py Core bug fix: corrects find_joints/find_bodies to return global indices when subset is provided, fixing issue #4439
source/isaaclab_newton/test/assets/utils/test_shared.py Comprehensive tests for the bug fix with excellent edge case coverage including non-contiguous subsets and order preservation
source/isaaclab_newton/isaaclab_newton/assets/articulation/articulation.py Updates find_bodies signature to add body_subset parameter for consistency with find_joints, plus various refactoring changes
source/isaaclab_newton/isaaclab_newton/assets/rigid_object/rigid_object.py Updates find_bodies signature to add body_subset parameter, plus extensive refactoring and improvements
source/isaaclab_newton/test/assets/articulation/test_articulation.py Adds integration tests for find_bodies and find_joints with subset parameters to validate the fix works end-to-end

Sequence Diagram

sequenceDiagram
    participant User
    participant Articulation
    participant find_joints
    participant resolve_matching_names
    
    User->>Articulation: find_joints(".*", joint_subset=["front_left_leg", "front_left_foot", "left_back_leg", "left_back_foot"])
    Articulation->>find_joints: Call with joint_names=[all 8 joints], subset=[4 joints]
    
    Note over find_joints: search_target = joint_subset (4 joints)
    
    find_joints->>resolve_matching_names: Match ".*" in subset
    resolve_matching_names-->>find_joints: indices=[0,1,2,3] (local to subset), names=[4 matched joints]
    
    Note over find_joints: Bug was here - returned local indices<br/>Fix: Map names back to global indices
    
    find_joints->>find_joints: indices = [joint_names.index(n) for n in names]
    Note over find_joints: New indices=[0,1,4,5] (global positions)
    
    find_joints->>find_joints: Create mask with len(joint_names)<br/>Set mask[0,1,4,5] = True
    
    find_joints-->>Articulation: Return (mask, names, [0,1,4,5])
    Articulation-->>User: Global indices [0,1,4,5] ✓
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.

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@kellyguo11
Copy link
Contributor

Could we try to capture some of the other changes in the PR description as well? or maybe we can try splitting up the PR a bit. github seems to be refusing to show more than 1 file at a time because of the PR size.

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

None yet

Development

Successfully merging this pull request may close these issues.

2 participants