Add Fan Wang's BPP Agent #10

kaikwan · 2025-07-13T20:54:00Z

num_envs = 1 only
Packing Action Cfg to accept obj bottom left origin placement transforms
Creating voxel grids from meshes for passing geometry to agent

- num_envs = 1 only - Packing Action Cfg to accept obj bottom left origin placement transforms - Creating voxel grids from meshes for passing geometry to agent

…ails

…improve performance

- Make BPP a class - Fix typos

…ogic to ensure indefinite runs

Copilot

Pull Request Overview

This PR integrates Fan Wang's Bin Packing Problem (BPP) agent based on "Stable bin packing of non-convex 3D objects with a robot manipulator", adding support for voxel-based geometry representation and configurable bottom-left placement origins.

Integration of Fan Wang's BPP agent with heuristic-based search packing
Configuration for bottom-left origin placement transforms in packing actions
Voxel grid creation from mesh geometry for passing to the BPP agent

Reviewed Changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tote_statistics.py	Code formatting improvements (quotes, parentheses, whitespace)
tote_manager.py	Added voxel support, refactored tote ejection logic, updated animation config
tote_helpers.py	Import reorganization and function signature formatting
pack_env_cfg.py	Enabled additional YCB objects and updated object counts
events.py	Added voxelized geometry computation and mesh properties caching
joint_pos_env_cfg.py	Added bottom-left placement configuration
wrappers_cfg.py	Import reorganization
wrappers.py	Import reorganization
packing_actions.py	Added bottom-left placement logic with bounding box calculations
actions_cfg.py	Added place_obj_bottomLeft configuration parameter
fanwang_bpp_agent.py	New BPP agent implementation
bpp_utils.py	New utility class for BPP functionality
Multiple script files	Import statement reorganization

Comments suppressed due to low confidence (2)

source/tote_consolidation/tote_consolidation/tasks/manager_based/pack/utils/tote_manager.py:61

The variable name '_tote_assets_state' is inconsistent with Python naming conventions. Consider using 'tote_assets_state' (without leading underscore) since it appears to be accessed from other classes.

        self._tote_assets_state = torch.stack([tote.get_world_poses()[0] for tote in self.tote_assets], dim=0)

source/tote_consolidation/tote_consolidation/tasks/manager_based/pack/mdp/events.py:97

The compute_voxelized_geometry function lacks proper docstring documentation. While it has parameter descriptions, it should include a proper docstring format with Args and Returns sections for consistency with other functions.

        points_attr = mesh.GetPointsAttr()

Copilot · 2025-07-15T21:36:26Z

source/tote_consolidation/tote_consolidation/tasks/manager_based/pack/utils/tote_manager.py

+            # Check if all objects in each tote are zero
+            empty_totes = torch.all(self.tote_to_obj[env_ids] == 0, dim=2)
+


This comment and the following lines (197-211) appear to be duplicated code from earlier in the function. The logic for checking empty totes and creating destination tote masks is repeated unnecessarily.

Suggested change

# Check if all objects in each tote are zero

empty_totes = torch.all(self.tote_to_obj[env_ids] == 0, dim=2)

Copilot · 2025-07-15T21:36:27Z

source/tote_consolidation/tote_consolidation/tasks/manager_based/pack/mdp/events.py

+        for point in points_local:
+            # Ensure indices are rounded properly (floor for point-to-voxel mapping)
+            voxel_index = torch.floor(point / voxel_size).long()
+            # Set voxel to 1 if within bounds
+            if (0 <= voxel_index).all() and (voxel_index < grid_size).all():
+                voxel_grid[tuple(voxel_index.tolist())] = 1.0


The loop iterating through all points to set voxel values is inefficient. Consider using vectorized operations with torch operations like torch.floor and boolean indexing to set multiple voxels at once.

Suggested change

for point in points_local:

# Ensure indices are rounded properly (floor for point-to-voxel mapping)

voxel_index = torch.floor(point / voxel_size).long()

# Set voxel to 1 if within bounds

if (0 <= voxel_index).all() and (voxel_index < grid_size).all():

voxel_grid[tuple(voxel_index.tolist())] = 1.0

# Compute voxel indices for all points

voxel_indices = torch.floor(points_local / voxel_size).long()

# Filter points within bounds

valid_mask = (voxel_indices >= 0).all(dim=1) & (voxel_indices < grid_size).all(dim=1)

valid_indices = voxel_indices[valid_mask]

# Update voxel grid for valid points

voxel_grid[valid_indices[:, 0], valid_indices[:, 1], valid_indices[:, 2]] = 1.0

Copilot · 2025-07-15T21:36:27Z

scripts/fanwang_bpp/bpp_utils.py

+            return self.get_action(env, new_packable_objects, tote_ids)
+
+        t = time.time()
+        obj_idx = obj_indicies[torch.randint(0, len(obj_indicies), (1,))][0]


Variable name 'obj_indicies' should be 'obj_indices' (correct spelling of 'indices').

Suggested change

obj_idx = obj_indicies[torch.randint(0, len(obj_indicies), (1,))][0]

obj_idx = obj_indices[torch.randint(0, len(obj_indices), (1,))][0]

Copilot · 2025-07-15T21:36:27Z

scripts/fanwang_bpp/bpp_utils.py

+    def get_action(self, env, obj_indicies, tote_ids):
+        """
+        Get the action for the current step in the packing problem.
+
+        Args:
+            obj_indicies: Indices of objects to consider


Parameter name 'obj_indicies' should be 'obj_indices' (correct spelling of 'indices').

Suggested change

def get_action(self, env, obj_indicies, tote_ids):

"""

Get the action for the current step in the packing problem.

Args:

obj_indicies: Indices of objects to consider

def get_action(self, env, obj_indices, tote_ids):

"""

Get the action for the current step in the packing problem.

Args:

obj_indices: Indices of objects to consider

Copilot · 2025-07-15T21:36:28Z

scripts/fanwang_bpp/bpp_utils.py

+        obj_indicies[env_idx] = [idx for idx in obj_indicies[env_idx] if idx != obj_idx]
+
+        return self.get_action(env, obj_indicies, tote_ids)


Variable name 'obj_indicies' should be 'obj_indices' (correct spelling of 'indices').

Suggested change

obj_indicies[env_idx] = [idx for idx in obj_indicies[env_idx] if idx != obj_idx]

return self.get_action(env, obj_indicies, tote_ids)

obj_indices[env_idx] = [idx for idx in obj_indices[env_idx] if idx != obj_idx]

return self.get_action(env, obj_indices, tote_ids)

…hs and filtering based on object IDs

…jection

…ckable objects keep getting retried

- Improved `ToteStatistics` to support incremental saving of statistics to a JSON file, including recent ejection data and operation counts.

…on to use filled voxel representations

…ulation

…nes and improve stats saving logic

…/GCULab into henri/baselines

Add Fan Wang's BPP Agent

d682199

- num_envs = 1 only - Packing Action Cfg to accept obj bottom left origin placement transforms - Creating voxel grids from meshes for passing geometry to agent

Copilot AI review requested due to automatic review settings July 13, 2025 20:54

This comment was marked as outdated.

Sign in to view

kaikwan added 4 commits July 13, 2025 14:06

Update README.md to include heuristic-scoring based packing agent det…

eb93b0c

…ails

Optimize object properties calculation by caching mesh properties to …

3bf98d2

…improve performance

Close-loop implementation of BPP

edcabf7

- Make BPP a class - Fix typos

Enhance BPP agent with unpackable object handling and tote ejection l…

8e5133d

…ogic to ensure indefinite runs

kaikwan requested a review from Copilot July 15, 2025 21:35

Copilot AI reviewed Jul 15, 2025

View reviewed changes

kaikwan and others added 14 commits July 15, 2025 14:44

Enhance pack environment configuration by dynamically loading USD pat…

279c279

…hs and filtering based on object IDs

Fix wrong indexing for removing objects from unpackable list during e…

345761e

…jection

Add stability checker for checking place pose candidates + Fixed unpa…

b97a297

…ckable objects keep getting retried

Refactor bpp utils for multiprocessing and enhance statistics logging

4a814ec

- Improved `ToteStatistics` to support incremental saving of statistics to a JSON file, including recent ejection data and operation counts.

Heightmap multiprocessing

dd11a68

Add seed argument to command line parser and configure environment seed

735b66f

Fix container not getting updated

1aabbf7

Add results parsing scripts and fixed object volume and GCU calculati…

29520e1

…on to use filled voxel representations

Refactor imports and clean up whitespace in various scripts

9161ce5

Fix summary and cummulative stats to same dir

f4ea308

Add comment to clarify asset position adjustment in bounding box calc…

0dd38cf

…ulation

Update README.md

7984f1c

Refactor tote manager and statistics handling: remove unnecessary clo…

cfd0ca2

…nes and improve stats saving logic

Merge branch 'henri/baselines' of https://github.com/personalrobotics…

95f38cc

…/GCULab into henri/baselines

kaikwan merged commit 71933b8 into main Jul 22, 2025

kaikwan deleted the henri/baselines branch July 22, 2025 19:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Fan Wang's BPP Agent #10

Add Fan Wang's BPP Agent #10

Uh oh!

kaikwan commented Jul 13, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 15, 2025

Uh oh!

Copilot AI Jul 15, 2025

Uh oh!

Copilot AI Jul 15, 2025

Uh oh!

Copilot AI Jul 15, 2025

Uh oh!

Copilot AI Jul 15, 2025

Uh oh!

Uh oh!

		# Check if all objects in each tote are zero
		empty_totes = torch.all(self.tote_to_obj[env_ids] == 0, dim=2)

-        for point in points_local:
-            # Ensure indices are rounded properly (floor for point-to-voxel mapping)
-            voxel_index = torch.floor(point / voxel_size).long()
-            # Set voxel to 1 if within bounds
-            if (0 <= voxel_index).all() and (voxel_index < grid_size).all():
-                voxel_grid[tuple(voxel_index.tolist())] = 1.0
+        # Compute voxel indices for all points
+        voxel_indices = torch.floor(points_local / voxel_size).long()
+        # Filter points within bounds
+        valid_mask = (voxel_indices >= 0).all(dim=1) & (voxel_indices < grid_size).all(dim=1)
+        valid_indices = voxel_indices[valid_mask]
+        # Update voxel grid for valid points
+        voxel_grid[valid_indices[:, 0], valid_indices[:, 1], valid_indices[:, 2]] = 1.0

	obj_idx = obj_indicies[torch.randint(0, len(obj_indicies), (1,))][0]
	obj_idx = obj_indices[torch.randint(0, len(obj_indices), (1,))][0]

		obj_indicies[env_idx] = [idx for idx in obj_indicies[env_idx] if idx != obj_idx]

		return self.get_action(env, obj_indicies, tote_ids)

Add Fan Wang's BPP Agent #10

Add Fan Wang's BPP Agent #10

Uh oh!

Conversation

kaikwan commented Jul 13, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!