Skip to content

Fixes CPU pipeline stage initialization in PhysxManager#4675

Merged
kellyguo11 merged 4 commits intoisaac-sim:developfrom
kellyguo11:fix-cpu-issue
Feb 24, 2026
Merged

Fixes CPU pipeline stage initialization in PhysxManager#4675
kellyguo11 merged 4 commits intoisaac-sim:developfrom
kellyguo11:fix-cpu-issue

Conversation

@kellyguo11
Copy link
Contributor

Description

In recent PhysicsManager refactoring, behavior in the PhysxManager changed to call attatch_stage by default for initialization.
This triggered a side effect in CPU pipeline that caused objects to miss contacts and collisions.

It appears that attach stage to PhysX before loading/starting is only needed for GPU pipeline. So in this change, we are guarding this call to run on GPU pipeline only and avoid it on CPU pipeline to avoid issues in collisions.

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 Feb 21, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 21, 2026

Greptile Summary

This PR fixes a CPU collision detection bug introduced during the PhysicsManager refactoring (#4564). The recent refactoring added an unconditional call to attach_stage() in the warmup process, which was intended to fix GPU pipeline initialization. However, this caused a double-initialization issue on the CPU pipeline where attach_stage() combined with force_load_physics_from_usd() corrupts the CPU broadphase (MBP) collision setup, causing objects to fall through surfaces non-deterministically.

Key changes:

  • Added device detection to check if GPU pipeline is in use (is_gpu = "cuda" in PhysicsManager.get_device())
  • Wrapped the attach_stage() call in a conditional that only executes for GPU pipeline
  • Added detailed comments explaining why this separation is necessary and the root cause of the CPU collision bug

The fix aligns with the original behavior where the old SimulationManager never called attach_stage() explicitly on CPU pipeline.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The fix is well-targeted and addresses a specific regression introduced in a recent refactoring. The change is minimal (7 lines modified), follows the same device detection pattern used elsewhere in the codebase (line 448), and includes clear explanatory comments. The fix restores the original CPU pipeline behavior while preserving the GPU pipeline fix.
  • No files require special attention

Important Files Changed

Filename Overview
source/isaaclab_physx/isaaclab_physx/physics/physx_manager.py Conditionally calls attach_stage() only for GPU pipeline to fix CPU collision detection issue caused by double-initialization of broadphase collision setup

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_warmup_and_create_views called] --> B{Check device type}
    B -->|GPU pipeline<br/>cuda in device| C[attach_stage]
    B -->|CPU pipeline<br/>cpu device| D[Skip attach_stage]
    C --> E[force_load_physics_from_usd]
    D --> E
    E --> F[start_simulation]
    F --> G[update_simulation]
    G --> H[Create SimulationView]
    H --> I[Physics Ready]
    
    C -.->|Prevents| J[GPU buffer allocation issues]
    D -.->|Prevents| K[CPU broadphase MBP<br/>double-initialization]
    K -.->|Would cause| L[Objects fall through surfaces]
Loading

Last reviewed commit: 90894d0

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.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@kellyguo11 kellyguo11 merged commit b5ad669 into isaac-sim:develop Feb 24, 2026
6 of 9 checks passed
matthewtrepte pushed a commit to matthewtrepte/IsaacLab that referenced this pull request Feb 24, 2026
# Description

In recent PhysicsManager refactoring, behavior in the PhysxManager
changed to call attatch_stage by default for initialization.
This triggered a side effect in CPU pipeline that caused objects to miss
contacts and collisions.

It appears that attach stage to PhysX before loading/starting is only
needed for GPU pipeline. So in this change, we are guarding this call to
run on GPU pipeline only and avoid it on CPU pipeline to avoid issues in
collisions.


## Type of change

<!-- As you go through the list, delete the ones that are not
applicable. -->

- 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
- [ ] 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

<!--
As you go through the checklist above, you can mark something as done by
putting an x character in it

For example,
- [x] I have done this task
- [ ] I have not done this task
-->
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