Skip to content

Conversation

@ooctipus
Copy link
Collaborator

Description

This PR adds a demo that showcasing the advance usage of RigidObjectCollection.
This shows spawning random number of objects and varying object types per reset with a bin packing example.

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

bin_packing

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 documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team labels Oct 22, 2025
@ooctipus ooctipus added enhancement New feature or request and removed documentation Improvements or additions to documentation labels Oct 22, 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

This PR introduces a new bin packing demo (scripts/demos/bin_packing.py) that demonstrates advanced RigidObjectCollection capabilities for spawning varying numbers of objects with different types across environments. The implementation uses a masking pattern to efficiently show/hide objects: active objects receive randomized poses above a bin while inactive ones are parked at a cached height with gravity disabled. This pattern showcases how RigidObjectCollection can handle dynamic object populations without destroying/recreating prims, which is valuable for robotics scenarios like warehouse bin picking where scene complexity varies. Documentation in showroom.rst follows the established pattern for demo scripts.

PR Description Notes:

  • Line 217 in showroom.rst: Typo in image alt text - "os" should be "of"

Potential Issues

  1. Position noise is non-functional (line 147-151): The POSE_RANGE dictionary only contains rotation keys ("roll", "pitch", "yaw") but the code attempts to retrieve position keys ("x", "y", "z") on line 148. This causes POSE_RANGE.get(key, (0.0, 0.0)) to return (0.0, 0.0) for all position dimensions, effectively disabling position noise injection. Either add position ranges to POSE_RANGE or remove the position noise logic.

  2. Object count range excludes maximum (line 278): torch.randint(MIN_OBJECTS_PER_BIN, num_objects, ...) excludes the upper bound, meaning if you want up to 24objects, the current code can only spawn 23. This should likely be num_objects + 1 to include the maximum count.

  3. Magic numbers reduce maintainability: The hardcoded pi approximations (-3.14, 3.14 on line 58) and the Z_POS constants (line 59) should use symbolic constants like math.pi and potentially be extracted into a configuration class to improve clarity and maintainability.

  4. Leftover debug comment (line 285): The commented-out print(f"Scene prim count: {scene.num_prims}") suggests incomplete cleanup from development.

  5. Timer message is misleading (line 282): The timer prints "Resetting scene state..." but it actually measures the entire simulation loop including state updates, resets, and rendering - not just the reset operation.


Confidence Score: 3/5

The demo effectively showcases the RigidObjectCollection masking pattern, but the non-functional position noise logic is a significant functional issue that undermines the stated goal of demonstrating "noisy spawns." The off-by-one issue in object count sampling also affects the demo's behavior. These issues suggest the code may not have been thoroughly tested against its intended behavior, though they don't prevent the demo from running.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 22, 2025
@kellyguo11 kellyguo11 moved this to In review in Isaac Lab Oct 22, 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

This PR adds a new bin packing demo (scripts/demos/bin_packing.py) that showcases advanced RigidObjectCollection functionality. The demo spawns 24 different grocery objects across multiple environments, dynamically randomizing which objects are active versus cached, and resets poses, velocities, and masses every 250 simulation steps. The implementation uses a caching mechanism that "parks" unused objects at an elevated position with gravity disabled rather than destroying/recreating them, demonstrating efficient object pool management. The demo integrates with the existing multi-asset spawning infrastructure shown in multi_asset.py and test_rigid_object_collection.py, extending the RigidObjectCollection patterns to handle varying object counts per environment reset. Documentation has been added to showroom.rst following the established demo documentation pattern.

PR Description Notes:

  • Minor typo in showroom.rst line 217: "os" should be "of" ("combination os" → "combination of")

Important Files Changed

Filename Score Overview
scripts/demos/bin_packing.py 2/5 New demo showcasing RigidObjectCollection with dynamic object spawning, but contains critical dictionary key mismatch bug on line 132 and off-by-one error on line 294
docs/source/overview/showroom.rst 4/5 Documentation addition for bin_packing demo following established pattern with minor typo in alt text

Confidence score: 2/5

  • This PR has critical bugs that will cause runtime failures and prevent the demo from functioning as intended
  • Score reflects two blocking issues: line 132 dictionary key mismatch (GROCERIES.get(f"OBJECT_{label}") won't find keys formatted as "OBJECT_CEREAL") and line 294 off-by-one error preventing maximum object spawning, plus unresolved position noise issue from previous review
  • Critical attention needed on scripts/demos/bin_packing.py lines 132-136 (dictionary key retrieval), line 294 (randint range), and lines 166-169 (POSE_RANGE position keys)

Sequence Diagram

sequenceDiagram
    participant User
    participant main
    participant AppLauncher
    participant SimulationContext
    participant InteractiveScene
    participant RigidObjectCollection
    participant run_simulator
    
    User->>main: "Execute bin_packing.py"
    main->>AppLauncher: "AppLauncher(args_cli)"
    AppLauncher-->>main: "simulation_app"
    
    main->>SimulationContext: "SimulationContext(sim_cfg)"
    SimulationContext-->>main: "sim"
    
    main->>SimulationContext: "set_camera_view((2.5, 0.0, 4.0), (0.0, 0.0, 2.0))"
    
    main->>InteractiveScene: "InteractiveScene(scene_cfg)"
    InteractiveScene->>RigidObjectCollection: "Create groceries collection"
    RigidObjectCollection-->>InteractiveScene: "groceries"
    InteractiveScene-->>main: "scene"
    
    main->>SimulationContext: "reset()"
    
    main->>run_simulator: "run_simulator(sim, scene)"
    
    loop Simulation Loop (while app is running)
        alt Every 250 steps (count % 250 == 0)
            run_simulator->>run_simulator: "Randomly choose num_active_groceries"
            run_simulator->>run_simulator: "Create groceries_mask (active/cached)"
            run_simulator->>reset_object_collections: "Reset cached objects"
            reset_object_collections->>RigidObjectCollection: "set_transforms(cached poses)"
            reset_object_collections->>RigidObjectCollection: "set_velocities(zero)"
            run_simulator->>reset_object_collections: "Reset active objects with noise"
            reset_object_collections->>RigidObjectCollection: "set_transforms(active poses + noise)"
            reset_object_collections->>RigidObjectCollection: "set_velocities(random)"
            run_simulator->>RigidObjectCollection: "set_masses(random_masses)"
            run_simulator->>RigidObjectCollection: "set_disable_gravities(cached objects)"
            run_simulator->>InteractiveScene: "reset()"
        end
        
        run_simulator->>InteractiveScene: "write_data_to_sim()"
        run_simulator->>SimulationContext: "step()"
        
        run_simulator->>RigidObjectCollection: "Get object_pos_w"
        RigidObjectCollection-->>run_simulator: "positions"
        run_simulator->>run_simulator: "Check out-of-bounds objects"
        
        alt Objects out of bounds
            run_simulator->>reset_object_collections: "Teleport back to active stack"
            reset_object_collections->>RigidObjectCollection: "set_transforms(active poses)"
            reset_object_collections->>RigidObjectCollection: "set_velocities(zero)"
        end
        
        run_simulator->>InteractiveScene: "update(sim_dt)"
    end
    
    run_simulator-->>main: "Return"
    main->>AppLauncher: "close()"
Loading

2 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

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

Added a bin packing demo (scripts/demos/bin_packing.py) that showcases advanced RigidObjectCollection usage with dynamic object spawning, caching, and randomization. The demo spawns a varying number of grocery objects per environment reset and manages them using active/cached states.

Key changes:

  • New demo script with interactive bin-packing simulation using YCB grocery objects
  • Documentation added to showroom with usage examples
  • Demonstrates spawning random numbers of objects with varying types per environment
  • Shows cache management, pose randomization, velocity randomization, and out-of-bounds recovery

Issues found:

  • Previous comments identified critical logic issues with .get() dictionary access and torch.randint upper bound
  • Documentation has a minor typo already flagged
  • Zero position noise due to missing x/y/z keys in POSE_RANGE was questioned but may be intentional

Confidence Score: 3/5

  • PR has several previously-identified logic issues that need resolution before merging
  • The demo adds valuable functionality but has critical bugs flagged in previous comments: .get() will always return None for the dictionary lookup (line 132), and torch.randint upper bound excludes the maximum value (line 294). These issues affect core functionality and need fixing.
  • scripts/demos/bin_packing.py requires attention for the .get() and randint issues; docs/source/overview/showroom.rst has a minor typo

Important Files Changed

File Analysis

Filename Score Overview
scripts/demos/bin_packing.py 3/5 New demo showcasing RigidObjectCollection with dynamic object spawning; has some issues with .get() usage and randint upper bound that were flagged in comments
docs/source/overview/showroom.rst 4/5 Added documentation entry for bin packing demo with command examples; has a typo ('os' should be 'of') already flagged

Sequence Diagram

sequenceDiagram
    participant Main
    participant SimContext as SimulationContext
    participant Scene as InteractiveScene
    participant Groceries as RigidObjectCollection
    participant PhysX as PhysX View

    Main->>SimContext: Initialize sim (dt=0.005)
    Main->>Scene: Create with MultiObjectSceneCfg
    Scene->>Groceries: Spawn groceries collection
    Main->>SimContext: reset()
    
    Main->>Main: build_grocery_defaults()
    Main->>Main: Setup active/cached spawn poses
    
    loop Every 250 steps
        Main->>Main: Generate random num_active_groceries
        Main->>Main: Create groceries_mask
        Main->>Main: reset_object_collections (cached objects)
        Main->>Main: reset_object_collections (active objects with noise)
        Main->>PhysX: set_masses (random)
        Main->>PhysX: set_disable_gravities (cached)
        Main->>Scene: reset()
    end
    
    loop Every step
        Main->>Scene: write_data_to_sim()
        Main->>SimContext: step()
        Main->>Groceries: Check out-of-bounds objects
        alt Objects out of bounds
            Main->>Main: reset_object_collections (teleport back)
        end
        Main->>Scene: update(sim_dt)
    end
Loading

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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

Labels

documentation Improvements or additions to documentation 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