Skip to content

Fixes incorrect device being used when reordering arrays in RigidObjectCollection#4940

Merged
kellyguo11 merged 3 commits intoisaac-sim:developfrom
AntoineRichard:antoiner/fix/fix_rigid_mock_failures
Mar 11, 2026
Merged

Fixes incorrect device being used when reordering arrays in RigidObjectCollection#4940
kellyguo11 merged 3 commits intoisaac-sim:developfrom
AntoineRichard:antoiner/fix/fix_rigid_mock_failures

Conversation

@AntoineRichard
Copy link
Copy Markdown
Collaborator

Description

_reshape_view_to_data_2d/_3d created strided views with device=self.device (GPU) on pointers from CPU arrays.

This could not be reproduced locally because our GPUs allowed for this using heterogeneous memory management, but the CI machine did no allow for this.

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

@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Mar 11, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 11, 2026

Greptile Summary

This PR fixes a device mismatch bug in RigidObjectCollectionData where _reshape_view_to_data_2d and _reshape_view_to_data_3d were creating Warp strided views with device=self.device (GPU) over raw pointers that actually point to CPU-allocated memory returned by PhysX (e.g. get_masses(), get_inertias()). On GPUs supporting heterogeneous memory management (HMM) this went unnoticed, but on standard CI hardware it caused failures.

Key changes:

  • Strided wp.array views are now created with device=data.device so the device tag correctly matches where the source pointer lives (CPU for PhysX mass/inertia data).
  • wp.clone(strided_view, self.device) now explicitly specifies the destination device, performing both the contiguity copy and the CPU→GPU transfer in a single call.
  • Helpful inline comments document why data.device is used (PhysX returns these arrays on CPU) to prevent the same regression in future.

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, targeted bug fix with no API surface changes.
  • The change is confined to two private helper methods, directly addresses the described root cause (CPU pointer misidentified as GPU memory), and follows the correct Warp idiom: create a strided alias on the source device, then clone to the target device. The other similar helpers (_get_pos_from_transform, etc.) are called with arrays already resident on self.device, so they are unaffected. No regressions are expected.
  • No files require special attention.

Important Files Changed

Filename Overview
source/isaaclab_physx/isaaclab_physx/assets/rigid_object_collection/rigid_object_collection_data.py Fixes incorrect device assignment in _reshape_view_to_data_2d and _reshape_view_to_data_3d: strided views now use data.device (CPU, matching PhysX output) before cloning to self.device, preventing pointer aliasing errors on machines without heterogeneous memory management.

Sequence Diagram

sequenceDiagram
    participant PhysX as PhysX (CPU)
    participant Helper as _reshape_view_to_data_2d/3d
    participant Warp as wp.array (strided view)
    participant GPU as self.device (GPU)

    PhysX->>Helper: get_masses() / get_inertias()<br/>(returns CPU wp.array)
    Note over Helper: BEFORE FIX: device=self.device (GPU)<br/>→ CPU ptr labelled as GPU → crash on non-HMM
    Helper->>Warp: wp.array(ptr=data.ptr, ..., device=data.device)<br/>Strided view stays on CPU
    Warp->>GPU: wp.clone(strided_view, self.device)<br/>Contiguous copy + CPU→GPU transfer
    GPU-->>Helper: Contiguous GPU wp.array
Loading

Last reviewed commit: 285a5c5

@kellyguo11 kellyguo11 merged commit 0c24c97 into isaac-sim:develop Mar 11, 2026
13 of 15 checks passed
daniela-hase pushed a commit to daniela-hase/IsaacLab that referenced this pull request Mar 30, 2026
…ctCollection (isaac-sim#4940)

# Description

_reshape_view_to_data_2d/_3d created strided views with
device=self.device (GPU) on pointers from CPU arrays.

This could not be reproduced locally because our GPUs allowed for this
using heterogeneous memory management, but the CI machine did no allow
for this.

## Type of change

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

## Checklist

- [x] I have read and understood the [contribution
guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

---------

Co-authored-by: Kelly Guo <kellyg@nvidia.com>
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