Skip to content

Conversation

@pascal-roth
Copy link
Collaborator

@pascal-roth pascal-roth commented Nov 3, 2025

Description

Remove dependency on IsaacSim stage_utils for integration of new simulation engines like newton.

Type of change

  • Dependency removal

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 enhancement New feature or request isaac-lab Related to Isaac Lab team labels Nov 3, 2025
@pascal-roth pascal-roth self-assigned this Nov 3, 2025
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.

Greptile Overview

Greptile Summary

Reimplemented IsaacSim's stage_utils module within IsaacLab to remove dependency on IsaacSim's stage utilities, enabling integration with new simulation engines like Newton.

Key Changes:

  • Created new isaaclab/sim/utils/stage.py module (795 lines) implementing stage utility functions previously imported from isaacsim.core.utils.stage
  • Restructured isaaclab/sim/utils.py to isaaclab/sim/utils/utils.py with corresponding __init__.py
  • Updated all 51 files across the codebase to import from local isaaclab.sim.utils.stage instead of isaacsim.core.utils.stage
  • Maintained backward compatibility with Isaac Sim versions < 5.0 through version checks
  • Preserved all existing functionality including stage-in-memory support, stage manipulation, and USD operations

Issue Found:

  • Critical import error in source/isaaclab/isaaclab/sim/utils/utils.py:35 - incorrect relative import path after file restructure (.spawners should be ..spawners)

Confidence Score: 2/5

  • This PR has a critical import error that will cause runtime failures
  • The PR successfully reimplements IsaacSim's stage utilities and updates imports across 51 files consistently. However, there is a critical syntax error in utils.py line 35 with an incorrect relative import path (.spawners instead of ..spawners) after the file was moved from sim/utils.py to sim/utils/utils.py. This will cause import failures at runtime when the TYPE_CHECKING guard is bypassed or when the module is actually imported.
  • Pay close attention to source/isaaclab/isaaclab/sim/utils/utils.py - the relative import on line 35 must be fixed before merge

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab/isaaclab/sim/utils/stage.py 4/5 new stage_utils implementation reimplementing IsaacSim functionality - comprehensive and follows original patterns closely
source/isaaclab/isaaclab/sim/utils/utils.py 2/5 moved from sim/utils.py to sim/utils/utils.py with incorrect relative import path on line 35
source/isaaclab/isaaclab/sim/simulation_context.py 5/5 updated imports to use local stage_utils instead of IsaacSim's version - clean refactor

Sequence Diagram

sequenceDiagram
    participant App as Application Code
    participant SimCtx as SimulationContext
    participant StageUtils as isaaclab.sim.utils.stage
    participant USD as USD Stage
    participant PhysX as PhysX Interface
    
    Note over App,PhysX: Before PR: Used isaacsim.core.utils.stage
    Note over App,PhysX: After PR: Uses isaaclab.sim.utils.stage
    
    App->>SimCtx: create_new_stage_in_memory()
    SimCtx->>StageUtils: create_new_stage_in_memory()
    StageUtils->>USD: Usd.Stage.CreateInMemory()
    USD-->>StageUtils: stage
    StageUtils-->>SimCtx: stage
    
    App->>StageUtils: use_stage(stage)
    Note over StageUtils: Sets thread-local context
    StageUtils->>StageUtils: _context.stage = stage
    
    App->>StageUtils: get_current_stage()
    StageUtils->>StageUtils: getattr(_context, "stage", ...)
    StageUtils-->>App: current stage
    
    App->>StageUtils: attach_stage_to_usd_context()
    StageUtils->>StageUtils: get_current_stage_id()
    StageUtils->>PhysX: attach_stage(stage_id)
    StageUtils->>SimCtx: skip_next_stage_open_callback()
    StageUtils->>USD: attach_stage_with_callback(stage_id)
    StageUtils->>PhysX: attach_stage(stage_id)
Loading

51 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Pascal Roth <[email protected]>
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.

Greptile Overview

Greptile Summary

Reimplements IsaacSim stage_utils as IsaacLab sim.utils.utils to remove external dependency for Newton engine integration.

Key Changes:

  • Adds new utils.py with USD prim manipulation utilities (1031 lines)
  • Implements decorators (@apply_nested, @clone) for prim operations
  • Provides material binding, export, and stage traversal functions
  • Handles Isaac Sim version compatibility (4.x vs 5.x)

Issues Found:

  • Line 292: Version parsing bug - attempts to join single string get_version()[2] which will cause runtime error

Confidence Score: 2/5

  • Contains critical version parsing bug that will cause runtime failures when cloning prims in Isaac Sim 5.0+
  • The version parsing bug on line 292 will cause an AttributeError when the clone decorator is used with multiple prim paths, breaking core spawning functionality. While the overall architecture is sound and the dependency removal goal is achieved, this syntax error needs immediate fixing before merge.
  • source/isaaclab/isaaclab/sim/utils/utils.py - fix line 292 version parsing before merge

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab/isaaclab/sim/utils/utils.py 3/5 New file reimplements IsaacSim stage_utils to remove dependency; contains version parsing bug on line 292

Sequence Diagram

sequenceDiagram
    participant User as User Code
    participant Utils as sim.utils.utils
    participant Stage as sim.utils.stage
    participant Spawners as sim.spawners
    participant IsaacSim as isaacsim.core

    Note over Utils: Removed IsaacSim stage_utils dependency
    
    User->>Utils: clone() decorator
    Utils->>Stage: get_current_stage()
    Stage->>IsaacSim: omni.usd context
    IsaacSim-->>Stage: USD Stage
    Stage-->>Utils: stage handle
    
    Utils->>Utils: find_matching_prim_paths()
    Utils->>Spawners: SpawnerCfg (TYPE_CHECKING)
    
    Utils->>IsaacSim: Cloner(stage)
    IsaacSim-->>Utils: cloner instance
    
    Utils->>IsaacSim: get_version()
    IsaacSim-->>Utils: version tuple
    Note over Utils: Bug: incorrect version parsing
    
    Utils->>IsaacSim: cloner.clone()
    IsaacSim-->>Utils: cloned prims
    Utils-->>User: spawned prim
Loading

Additional Comments (1)

  1. source/isaaclab/isaaclab/sim/utils/utils.py, line 292 (link)

    syntax: incorrect version parsing - get_version()[2] returns a single string (e.g. "5"), cannot join it

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@pascal-roth pascal-roth moved this to In review in Isaac Lab Nov 3, 2025
@ooctipus
Copy link
Collaborator

ooctipus commented Nov 5, 2025

Seems like many things are failing 👀

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.

Greptile Overview

Greptile Summary

This PR refactors the import statement in visual_materials.py to consolidate imports from isaaclab.sim.utils.stage to use the unified isaaclab.sim.utils module interface instead.

Key Changes:

  • Changed from isaaclab.sim.utils.stage import to from isaaclab.sim.utils import for attach_stage_to_usd_context, clone, and safe_set_attribute_on_usd_prim
  • This change aligns with the established pattern used across the codebase where utils/__init__.py re-exports all utilities
  • The functionality remains identical as utils.py imports and re-exports attach_stage_to_usd_context from stage.py

Context:
The PR description mentions removing dependency on IsaacSim stage_utils for integration with new simulation engines like Newton. This import consolidation is part of a larger refactoring effort to make the stage utilities implementation-agnostic by exposing them through a unified interface rather than directly from the stage module.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change is a simple import path refactoring that maintains identical functionality. The functions being imported (attach_stage_to_usd_context, clone, safe_set_attribute_on_usd_prim) are properly re-exported through the utils module's __init__.py, making this a safe reorganization that improves consistency with the rest of the codebase
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab/isaaclab/sim/spawners/materials/visual_materials.py 5/5 Updated import to use isaaclab.sim.utils instead of isaaclab.sim.utils.stage for consistency with codebase patterns

Sequence Diagram

sequenceDiagram
    participant Client as Client Code
    participant VM as visual_materials.py
    participant Utils as sim.utils module
    participant Stage as stage.py
    participant USDPrim as USD Prim
    
    Client->>VM: spawn_preview_surface(prim_path, cfg)
    VM->>Utils: import attach_stage_to_usd_context
    Utils->>Stage: re-export from stage.py
    VM->>Stage: attach_stage_to_usd_context(attaching_early=True)
    Stage->>Stage: check if stage in memory
    Stage->>USDPrim: attach to USD context if needed
    VM->>Utils: import clone decorator
    VM->>Utils: import safe_set_attribute_on_usd_prim
    VM->>USDPrim: create material prim and set attributes
    VM-->>Client: return created prim
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@ooctipus
Copy link
Collaborator

ooctipus commented Nov 7, 2025

I looked into the failing issue it seems like because we are combining from utils import * with cross package import. If you changes import isaaclab.sim.utils.stage as stage_utils to from isaaclab.sim.utils import stage as stage_utils

it will work again,
Explaination:

from utils import * rebinds names on the parent package, and in your case it ends up binding isaaclab.sim.utils to the module isaaclab.sim.utils.utils instead of the utils package.
import isaaclab.sim.utils.stage as stage_utils requires isaaclab.sim.utils to be a package (with path) so it can descend to stage, which fails when it’s actually the module.
from isaaclab.sim.utils import stage as stage_utils works because it imports the package and then accesses its stage attribute directly, bypassing that package-versus-module mix-up.

Personally I think that the root cause is import * in init.py files. It blindly rebinds names and can shadow the package object (isaaclab.sim.utils) with a module (utils.py), breaking dotted imports like import isaaclab.sim.utils.stage as ….

In the new rework we may need to consider to drop * import all together (at least for core)

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

Labels

enhancement New feature or request isaac-lab Related to Isaac Lab team

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

2 participants