Skip to content

Fix croodinate pickment #514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
skywhite1024 merged 1 commit into from
Aug 14, 2026
Merged

Fix croodinate pickment #514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions embodichain/data/assets/obj_assets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,3 +240,31 @@ def __init__(self, data_root: str = None):
path = EMBODICHAIN_DEFAULT_DATA_ROOT if data_root is None else data_root

super().__init__(prefix, data_descriptor, path)

@greptile-apps greptile-apps Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Public asset exports remain implicit

PlasticTray and WaterBasin are new public dataset classes, but the module does not declare them through __all__ as required for public modules. Because the assets package imports this module with a wildcard, its exported API remains dependent on implementation-level names rather than an explicit declaration.

Context Used: CLAUDE.md (source)

Prompt To Fix With AI 
This is a comment left during a code review.
Path: embodichain/data/assets/obj_assets.py
Line: 243

Comment:
**Public asset exports remain implicit**

`PlasticTray` and `WaterBasin` are new public dataset classes, but the module does not declare them through `__all__` as required for public modules. Because the assets package imports this module with a wildcard, its exported API remains dependent on implementation-level names rather than an explicit declaration.

**Context Used:** CLAUDE.md ([source](https://github.com/dexforce/embodichain/blob/main/CLAUDE.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex Fix in Claude Code


class PlasticTray(EmbodiChainDataset):
"""get_data_path("PlasticTray/plastic_tray.glb")"""

def __init__(self, data_root: str = None):
data_descriptor = o3d.data.DataDescriptor(
os.path.join(EMBODICHAIN_DOWNLOAD_PREFIX, obj_assets, "PlasticTray.zip"),
"66f1f8a507052f9e33be5433fa2a2667",
)
prefix = type(self).__name__
path = EMBODICHAIN_DEFAULT_DATA_ROOT if data_root is None else data_root

super().__init__(prefix, data_descriptor, path)


class WaterBasin(EmbodiChainDataset):
"""get_data_path("WaterBasin/water_basin.glb")"""

def __init__(self, data_root: str = None):
data_descriptor = o3d.data.DataDescriptor(
os.path.join(EMBODICHAIN_DOWNLOAD_PREFIX, obj_assets, "WaterBasin.zip"),
"9ae41630f6f52dccd7b95ab21b6ba989",
)
prefix = type(self).__name__
path = EMBODICHAIN_DEFAULT_DATA_ROOT if data_root is None else data_root

super().__init__(prefix, data_descriptor, path)
3 changes: 2 additions & 1 deletion embodichain/toolkits/graspkit/pg_grasp/antipodal_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
Path.home() / ".cache" / "embodichain" / "grasp_annotator_cache"
)
GRASP_ANNOTATOR_CACHE_DIR.mkdir(parents=True, exist_ok=True)
VERSION_TAG = "v0.0.1"


__all__ = ["GraspGenerator", "GraspGeneratorCfg"]
Expand Down Expand Up @@ -419,7 +420,7 @@ def _get_cache_dir(self, vertices: torch.Tensor, triangles: torch.Tensor):
face_bytes = triangles.to("cpu").numpy().tobytes()
md5_hash = hashlib.md5(vert_bytes + face_bytes).hexdigest()
cache_path = os.path.join(
GRASP_ANNOTATOR_CACHE_DIR, f"antipodal_cache_{md5_hash}.npy"
GRASP_ANNOTATOR_CACHE_DIR, f"antipodal_cache_{VERSION_TAG}_{md5_hash}.npy"
)
return cache_path

Expand Down
11 changes: 7 additions & 4 deletions embodichain/toolkits/graspkit/pg_grasp/antipodal_sampler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,14 @@ def sample(self, vertices: torch.Tensor, faces: torch.Tensor) -> torch.Tensor:
ray_origin = (
sample_points - 2.0 * max_range * ray_direc
) # ray origin in the other side of the mesh
# casting
ray_origin_2 = sample_points - 2.0 * self.cfg.max_length * ray_direc
all_ray_origin = torch.cat([ray_origin, ray_origin_2], dim=0)
all_ray_direc = torch.cat([ray_direc, ray_direc], dim=0)
all_surface_origin = torch.cat([sample_points, sample_points], dim=0)
return self._get_raycast_result(
Comment on lines +94 to 98
ray_origin,
ray_direc,
surface_origin=sample_points,
all_ray_origin,
all_ray_direc,
surface_origin=all_surface_origin,
)

def _sample_surface_by_fibonacci_raycast(
Expand Down
31 changes: 28 additions & 3 deletions scripts/tutorials/atomic_action/coordinated_pickment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import torch

from embodichain.lab.sim import SimulationManager
from embodichain.data import get_data_path
from embodichain.lab.sim.atomic_actions import (
ActionBinding,
ActionInvocation,
Expand Down Expand Up @@ -80,8 +81,8 @@

PICKMENT_ASSET_ROOT = "CoordinatedPlacementAndPickment"
GRIPPER_TCP_Z = 0.121
SUPPORT_SURFACE_Z = 0.65
SUPPORT_SURFACE_SIZE = (0.60, 0.60, 0.02)
SUPPORT_SURFACE_Z = 0.55
SUPPORT_SURFACE_SIZE = (0.7, 1.20, 0.02)
SUPPORT_SURFACE_CENTER = (
0.0,
0.0,
Expand Down Expand Up @@ -133,6 +134,28 @@ class PickmentObjectPreset:
target_world_yaw_deg=0.0,
hand_close_qpos=0.026,
),
"water_basin": PickmentObjectPreset(
label="water_basin",
mesh_path=get_data_path("WaterBasin/water_basin.glb"),

@greptile-apps greptile-apps Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Assets resolve during module import

If either new asset is absent from the local cache and the download service is unavailable, constructing OBJECT_PRESETS invokes get_data_path before argument parsing and aborts the import. This prevents the tutorial from running even when the user selects an existing local object such as pencil that does not require either new asset.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: scripts/tutorials/atomic_action/coordinated_pickment.py
Line: 139

Comment:
**Assets resolve during module import**

If either new asset is absent from the local cache and the download service is unavailable, constructing `OBJECT_PRESETS` invokes `get_data_path` before argument parsing and aborts the import. This prevents the tutorial from running even when the user selects an existing local object such as `pencil` that does not require either new asset.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex Fix in Claude Code

init_xy=(0.0, 0.02),
init_rot=(0.0, 0.0, 0.0),
surface_clearance=0.008,
body_scale=(1.0, 1.0, 1.0),
target_translation=(-0.12, -0.03, 0.12),
target_world_yaw_deg=0.0,
hand_close_qpos=0.026,
),
"plastic_tray": PickmentObjectPreset(
label="plastic_tray",
mesh_path=get_data_path("PlasticTray/plastic_tray.glb"),
init_xy=(-0.02, 0.02),
init_rot=(0.0, 0.0, 90.0),
surface_clearance=0.008,
body_scale=(1.0, 1.0, 1.0),
target_translation=(-0.12, -0.03, 0.12),
target_world_yaw_deg=0.0,
hand_close_qpos=0.026,
),
}
PICKMENT_SAMPLE_INTERVAL = 96
PICKMENT_OBJECT_MOTION_KEYFRAMES = 6
Expand Down Expand Up @@ -160,7 +183,7 @@ def parse_arguments() -> argparse.Namespace:
parser.add_argument(
"--object",
choices=sorted(OBJECT_PRESETS),
default="pencil",
default="plastic_tray",
help="Object mesh to grasp in the coordinated pickment demo.",
)
return parser.parse_args()
Expand Down Expand Up @@ -355,6 +378,7 @@ def run_coordinated_pickment_demo(
obj,
label=preset.label,
n_sample=args.n_sample,
# n_sample = 1000,
force_reannotate=args.force_reannotate,
Comment on lines 379 to 382
)
left_to_right_arm_direction = compute_left_to_right_arm_direction(robot, sim.device)
Expand All @@ -377,6 +401,7 @@ def run_coordinated_pickment_demo(
hold_steps=PICKMENT_HOLD_STEPS,
object_motion_keyframes=PICKMENT_OBJECT_MOTION_KEYFRAMES,
left_to_right_arm_direction=left_to_right_arm_direction,
middle_empty_ratio=0.7,
)
engine = AtomicActionEngine(
motion_generator=motion_gen,
Expand Down
20 changes: 15 additions & 5 deletions scripts/tutorials/atomic_action/tutorial_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@
GRIPPER_HAND_JOINT_PATTERN = "gripper_finger1_joint_1"
GRIPPER_TCP_Z = 0.15
GRIPPER_MAX_OPEN_WIDTH = 0.100
GRIPPER_FINGER_LENGTH = 0.12
GRIPPER_MIN_OPEN_WIDTH = 0.003
GRIPPER_FINGER_LENGTH = 0.10
GRIPPER_ROOT_Z_WIDTH = 0.096
GRIPPER_Y_THICKNESS = 0.040
DEFAULT_GRIPPER_CLOSE_QPOS = 0.024
Expand Down Expand Up @@ -224,6 +225,7 @@ def run_tutorial(main: Callable[[], None]) -> None:
def add_ur5_gripper_robot(
sim: SimulationManager,
init_pos: Sequence[float] = (0.0, 0.0, 0.0),
init_qpos: Sequence[float] | None = None,
) -> Robot:
Comment on lines 226 to 229
"""Add the standard UR5 plus PGI gripper tutorial robot.

Expand All @@ -234,7 +236,9 @@ def add_ur5_gripper_robot(
Returns:
The added robot instance.
"""
return sim.add_robot(cfg=create_ur5_gripper_robot_cfg(init_pos=init_pos))
return sim.add_robot(
cfg=create_ur5_gripper_robot_cfg(init_pos=init_pos, init_qpos=init_qpos)
)


def create_toppra_motion_generator(robot: Robot) -> MotionGenerator:
Expand Down Expand Up @@ -308,15 +312,15 @@ def create_antipodal_semantics(
finger_length=GRIPPER_FINGER_LENGTH,
y_thickness=GRIPPER_Y_THICKNESS,
root_z_width=GRIPPER_ROOT_Z_WIDTH,
open_check_margin=0.002,
open_check_margin=0.03,
point_sample_dense=0.012,
),
generator_cfg=GraspGeneratorCfg(
viser_port=11801,
antipodal_sampler_cfg=AntipodalSamplerCfg(
n_sample=n_sample,
max_length=GRIPPER_MAX_OPEN_WIDTH,
min_length=0.005,
min_length=GRIPPER_MIN_OPEN_WIDTH,
),
is_partial_annotate=False,
is_filter_ground_collision=False,
Expand Down Expand Up @@ -749,6 +753,7 @@ def clone_local_pose_from_first_env(entity) -> torch.Tensor:

def create_ur5_gripper_robot_cfg(
init_pos: Sequence[float] = (0.0, 0.0, 0.0),
init_qpos: Sequence[float] | None = None,
) -> RobotCfg:
"""Build a UR5 arm + DH_PGI_140_80 gripper robot configuration.

Expand All @@ -773,6 +778,11 @@ def create_ur5_gripper_robot_cfg(
Returns:
A fully populated :class:`~embodichain.lab.sim.cfg.RobotCfg`.
"""
qpos = (
[0.0, -1.57, 1.57, -1.57, -1.57, 0.0, 0.0, 0.0]
if init_qpos is None
else list(init_qpos)
)
Comment on lines +781 to +785
return URRobotCfg.from_dict(
{
"robot_type": "ur5",
Expand Down Expand Up @@ -809,7 +819,7 @@ def create_ur5_gripper_robot_cfg(
]
}
},
"init_qpos": [0.0, -1.57, 1.57, -1.57, -1.57, 0.0, 0.0, 0.0],
"init_qpos": qpos,
"init_pos": init_pos,
}
)
Expand Down
Loading