IsaacLab icon indicating copy to clipboard operation
IsaacLab copied to clipboard

Replaces IsaacSim `prim_utils` with IsaacLab `prim_utils`

Open pascal-roth opened this issue 1 month ago • 2 comments

Description

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

NOTE: this PR depends on #3923

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

pascal-roth avatar Nov 03 '25 10:11 pascal-roth

Looks like tests are failing :eyes:

ooctipus avatar Nov 05 '25 23:11 ooctipus

Greptile Overview

Greptile Summary

This PR successfully replaces IsaacSim's prim_utils with a new IsaacLab implementation to remove the dependency and enable integration of new simulation engines like Newton.

Key Changes

  • New isaaclab.sim.utils.prims module: Implements ~1000 lines of prim utility functions previously provided by IsaacSim, including get_prim_at_path, create_prim, delete_prim, is_prim_path_valid, and many others
  • New isaaclab.sim.utils.semantics module: Implements semantic label management using UsdSemantics.LabelsAPI, including add_labels, get_labels, remove_labels, and upgrade utilities
  • Import refactoring: Updated 58 files to import from isaaclab.sim.utils.prims instead of isaacsim.core.utils.prims
  • Function migration: find_matching_prim_paths moved from prim_utils to sim_utils, with string utilities moved to isaaclab.utils.string
  • Circular import fix: Fixed circular dependency in sim/utils/utils.py by using lazy import for schemas module

Architecture Impact

The refactoring maintains backward compatibility at the API level - all existing function signatures remain unchanged. The new implementation still depends on IsaacSim for core functionality (XFormPrim, SimulationContext) but removes the dependency on IsaacSim's prim utilities layer, providing more control for future engine integrations.

Confidence Score: 3/5

  • This PR is relatively safe to merge but requires testing due to eval() usage and large-scale refactoring
  • Score reflects: (1) use of eval() with string concatenation in set_prim_attribute_value (lines 192, 194) which poses a security concern despite controlled input dictionary, (2) extensive refactoring across 58 files increasing regression risk, (3) new ~1000-line implementation of critical prim utilities that needs thorough testing, (4) circular import fix suggests complexity in module dependencies. The refactoring is well-structured and imports are consistently updated, but the eval() usage and scale of changes warrant careful testing before merge.
  • Pay close attention to source/isaaclab/isaaclab/sim/utils/prims.py (eval usage, new implementation) and ensure thorough integration testing across all spawner modules

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab/isaaclab/sim/utils/prims.py 3/5 New prim_utils implementation replacing IsaacSim dependency. Contains eval() usage with string concatenation (line 192, 194) for type mapping - security concern already noted.
source/isaaclab/test/sim/test_utils.py 5/5 Import replacement and removed test for find_matching_prim_paths (now in sim_utils)
source/isaaclab/isaaclab/sim/utils/utils.py 5/5 Fixed circular import by using lazy import for schemas module and removed top-level schemas import
source/isaaclab/isaaclab/utils/string.py 5/5 Added utility functions find_unique_string_name and find_root_prim_path_from_regex (moved from IsaacSim prim_utils)
source/isaaclab/isaaclab/sim/utils/semantics.py 5/5 New semantics utilities implementation replacing IsaacSim dependency for semantic label handling
source/isaaclab/isaaclab/sim/spawners/shapes/shapes.py 5/5 Import replacement from isaacsim.core.utils.prims to isaaclab.sim.utils.prims

Sequence Diagram

sequenceDiagram
    participant User as User Code
    participant Spawners as Spawners<br/>(shapes, meshes, etc.)
    participant PrimUtils as isaaclab.sim.utils.prims
    participant SimUtils as isaaclab.sim.utils
    participant Semantics as isaaclab.sim.utils.semantics
    participant IsaacSim as IsaacSim Core<br/>(XFormPrim, SimulationContext)
    participant USD as USD Stage

    User->>Spawners: spawn_cone(cfg)
    Spawners->>PrimUtils: create_prim(path, type, ...)
    PrimUtils->>PrimUtils: define_prim(path, type)
    PrimUtils->>USD: DefinePrim()
    USD-->>PrimUtils: Usd.Prim
    
    alt semantic_label provided
        PrimUtils->>Semantics: add_labels(prim, labels)
        Semantics->>USD: Apply UsdSemantics.LabelsAPI
    end
    
    alt transformations provided
        PrimUtils->>IsaacSim: XFormPrim(path, positions, ...)
        IsaacSim->>USD: Apply transforms
    end
    
    PrimUtils-->>Spawners: Usd.Prim
    
    alt cloning needed
        Spawners->>SimUtils: clone(func)(cfg, prim_path)
        SimUtils->>SimUtils: find_matching_prim_paths(regex)
        SimUtils->>USD: Traverse and match paths
        USD-->>SimUtils: matched paths
        SimUtils->>USD: Clone prims
    end
    
    Spawners-->>User: spawned prims

greptile-apps[bot] avatar Nov 11 '25 00:11 greptile-apps[bot]

@ooctipus lets get that one merged too

pascal-roth avatar Nov 19 '25 01:11 pascal-roth

the package import for prim_utils doesn't work for the same reason as stage_utils, we used too many wild card import isaaclab.sim.utils.prims as prim_utils

need to change to from isaaclab.sim.utils import prims as prim_utils so python can correctly recognized it as module

ooctipus avatar Nov 19 '25 10:11 ooctipus

the package import for prim_utils doesn't work for the same reason as stage_utils, we used too many wild card

import isaaclab.sim.utils.prims as prim_utils

need to change to from isaaclab.sim.utils import prims as prim_utils so python can correctly recognized it as module

That's not great, let's split up the utils/utils.py now to enable those imports again

pascal-roth avatar Nov 19 '25 13:11 pascal-roth

behavior change for a util function, has to be checked before merg

pascal-roth avatar Nov 20 '25 19:11 pascal-roth

@pascal-roth Thanks for putting this together. I’m not fully convinced these one-line wrappers add much value, though. Since USD already has solid documentation, I wonder if maintaining parallel Isaac Sim functions might increase overhead without clear benefit. Would it make sense to remove these helpers and reference USD directly instead?

Personally, I’d prefer removing the wrappers altogether, but I’m fine handling that either here or in a follow-up MR depending on what keeps the review simplest.

Mayankm96 avatar Dec 01 '25 10:12 Mayankm96

I agree is Mayank, my personal opinion is to remove this wrapper in follow up PR, as thorough removal require more thorough refactor, this pr is more likely what will lands in newton for the only purpose of remove kit dependency, and we make sure the main has something similar.

ooctipus avatar Dec 01 '25 21:12 ooctipus

sounds like we are mostly ok with this PR to go in for now to be in alignment with the newton branch, but we should look into cleaning up all of the utilities in a follow-up PR. I think we should get this merged in then so that we also do not block the multi-mesh raycaster PR.

@Mayankm96 @ooctipus @AntoineRichard are you guys ok to give approval to merge this?

kellyguo11 avatar Dec 03 '25 21:12 kellyguo11