IsaacLab icon indicating copy to clipboard operation
IsaacLab copied to clipboard

Refactors environment cloner to directly work with physx and usd apis, removing isaac-sim cloner dependency

Open ooctipus opened this issue 1 month ago • 2 comments

Description

This PR refactors cloner logic into usd_replicate, physx_replicate, (also test to work for newton_replicate but not include in this PR)

At the highest level, This PR decouples cloning responsibility away from spawner, and delegate it to a dedicated lab cloner. The only job for spawner is to spawn prototype, and if any cloning is intended, one needs to use the cloner(either USD cloner, Physx Cloner, or Newton Cloner, whether that be homogeneous cloning or heterogeneous cloning).

In detail this PR

  • Adds heterogeneous cloning to InteractiveScene with per-object replication when envs differ, improving PhysX cloning time for heterogeneous scenes while perserving whole env replication in homogenous setting.

  • Introduces scene-level flag InteractiveSceneCfg.random_heterogenous_cloning (defaults True) to control prototype assignment: ---True: randomly assigns a prototype per env. ---False: uses round‑robin (env_index % num_prototypes) for determinism (previous behavior).

  • Removes hard dependency on the Isaac Sim Cloner; uses internal replicators usd_replicate and physx_replicate. Deprecates random_choice in MultiAssetSpawnerCfg and MultiUsdFileCfg in favor of the scene-level flag for consistent, scalable control.

While this PR tries to preserve as much original behavior as possible, with new refactors, it makes cloning with explicit constraints and composition, even proportion very easy, and I will be working on that as next goal.

Perf

Runing scripts/demos/multi_asset.py with 8192 envs

Before:

[INFO] Time to create scene: : 29.790888 seconds [INFO] Time to randomize scene: : 0.161381 seconds [INFO] to start Simulation: : 27.550733 seconds

After:

[INFO] Time to create scene: : 31.276912 seconds [INFO] Time to randomize scene: : 0.004585 seconds [INFO] Time to start Simulation: : 5.232237 seconds <--------------- much quicker

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (existing functionality will not work without user modification)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • [x] I have read and understood the contribution guidelines
  • [x] I have run the pre-commit checks with ./isaaclab.sh --format
  • [ ] 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

ooctipus avatar Nov 05 '25 07:11 ooctipus

This PR requires discussion with team

ooctipus avatar Nov 05 '25 21:11 ooctipus

Greptile Overview

Greptile Summary

This PR refactors environment cloning to remove the Isaac Sim GridCloner dependency and implement custom USD and PhysX replication. The changes decouple cloning from spawning, introduce a new cloner.py module with usd_replicate and physx_replicate functions, and add heterogeneous cloning support with per-object replication.

Key Changes

  • New source/isaaclab/isaaclab/scene/cloner.py module implementing TemplateCloneCfg, clone_from_template(), usd_replicate(), and physx_replicate()
  • InteractiveScene refactored to use new cloner instead of Isaac Sim GridCloner
  • Spawners simplified to only create prototypes under /World/template path - no longer handle cloning
  • Added random_heterogenous_cloning config flag to control prototype assignment (random vs round-robin)
  • Deprecated random_choice in MultiAssetSpawnerCfg and MultiUsdFileCfg
  • Performance improvement: simulation start time reduced from 27.5s to 5.2s (81% faster) for 8192 envs

Critical Bug Found

cloner.py:154 - replicate_args undefined when clone_usd=False and clone_physx=True, causing NameError. This configuration is valid but will crash at runtime. The variable is only defined inside the if cfg.clone_usd: blocks (lines 141-152) but used unconditionally in the PhysX replication section.

Other Issues

  • Missing test coverage for clone_usd=False + clone_physx=True configuration that would catch the critical bug
  • The bug was flagged multiple times in previous comments but remains unfixed in the latest commit

Confidence Score: 1/5

  • Critical bug will cause runtime crashes in valid configurations - not safe to merge
  • Score of 1 reflects critical NameError in cloner.py:154 that will crash when users configure clone_usd=False with clone_physx=True. This is a valid configuration per the API but produces undefined variable access. Bug was flagged in multiple previous comments but remains unfixed.
  • Pay critical attention to source/isaaclab/isaaclab/scene/cloner.py - fix the NameError before merging

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab/isaaclab/scene/cloner.py 1/5 New file implementing custom cloning - contains critical NameError bug when clone_usd=False and clone_physx=True (line 154)
source/isaaclab/isaaclab/scene/interactive_scene.py 3/5 Refactored to use new cloner module - removed Isaac Sim GridCloner dependency, simplified cloning logic
source/isaaclab/isaaclab/sim/spawners/wrappers/wrappers.py 4/5 Simplified spawner to create prototypes only - removed cloning logic and carb settings dependency
source/isaaclab/test/sim/test_cloner.py 3/5 New test file for cloner module - missing test case for clone_usd=False + clone_physx=True configuration

Sequence Diagram

sequenceDiagram
    participant User
    participant InteractiveScene
    participant Spawner
    participant Cloner
    participant USD_Stage
    participant PhysX

    User->>InteractiveScene: __init__(cfg)
    InteractiveScene->>InteractiveScene: create cloner_cfg
    InteractiveScene->>USD_Stage: create /World/template
    InteractiveScene->>USD_Stage: create env_0 prim
    InteractiveScene->>Cloner: usd_replicate(env_0 to env_1..N)
    Cloner->>USD_Stage: replicate empty env prims
    
    InteractiveScene->>InteractiveScene: _add_entities_from_cfg()
    loop for each asset config
        InteractiveScene->>Spawner: spawn asset
        Note over Spawner: prototype path = /World/template/.../proto_asset_.*
        Spawner->>USD_Stage: spawn prototypes (proto_asset_0, proto_asset_1, ...)
        Spawner-->>InteractiveScene: return prim
    end

    InteractiveScene->>InteractiveScene: clone_environments()
    
    alt replicate_physics=True
        InteractiveScene->>Cloner: clone_from_template(clone_physx=True)
        Cloner->>Cloner: discover prototypes
        Cloner->>Cloner: build mapping (random or modulo)
        Cloner->>Cloner: check if all envs use proto_0
        
        alt homogeneous (all proto_0)
            Cloner->>Cloner: prepare whole-env replication args
            opt clone_usd=True
                Cloner->>USD_Stage: usd_replicate entire env_0
            end
            Cloner->>PhysX: physx_replicate entire env_0
        else heterogeneous (mixed prototypes)
            Cloner->>Cloner: prepare per-object replication args
            opt clone_usd=True
                Cloner->>USD_Stage: usd_replicate per object
            end
            Cloner->>PhysX: physx_replicate per object
            Note over Cloner,PhysX: BUG: replicate_args undefined if clone_usd=False!
        end
    else replicate_physics=False
        InteractiveScene->>Cloner: clone_from_template(clone_physx=False)
        Cloner->>USD_Stage: usd_replicate only
    end
    
    opt filter_collisions=True
        InteractiveScene->>Cloner: filter_collisions()
        Cloner->>PhysX: setup collision groups
    end
    
    InteractiveScene-->>User: scene ready

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