Feature/add tong simulator

[yuzzy5828]

Tong System 環境とcameraをつけた

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR adds a new "Tong System" robot (a dual-arm system based on UR5 kinematics with custom tong end-effectors) to Isaac Lab, along with lift task environments, depth camera observation support, and Docker infrastructure changes to integrate a custom rsl_rl fork. The contribution is substantial in scope but has several issues that need to be addressed before merging — most critically, a malformed URDF that will fail to parse, hardcoded container paths that break non-Docker workflows, and Docker build context changes that break the existing build for all users.

Design Assessment

The robot and task configuration follow Isaac Lab conventions well — the pattern of creating a robot config in isaaclab_assets, registering gym environments, and extending LiftEnvCfg mirrors the existing Franka lift task. However, two design choices are problematic:

1. Docker infrastructure changes are too invasive. Changing the Docker build context from ../ to ../../ and requiring rsl_rl as a sibling directory breaks the existing setup for every user. This should be handled via a separate Dockerfile or docker-compose override — not by modifying the shared base configuration.

2. Dependency on external OnPolicyRunnerWithDepth — This class doesn't exist in the official rsl_rl package. Modifying the shared train.py to import from a custom fork creates a hard dependency that will cause ImportError for anyone using the standard rsl_rl.

Findings

🔴 Critical: Malformed XML in tong_system.urdf — source/isaaclab_assets/isaaclab_assets/robots/urdf_for_usd/tong_system/tong_system.urdf:761

Line 761 contains -<origin rpy="0.0 0.0 0" xyz="0.173 0 -0.04150"/> — the leading - before <origin is invalid XML. This will cause the URDF parser to fail when loading the tong system robot. Additionally, line 1 has a leading space before the XML declaration (<?xml ...>), which violates the XML spec.

    <origin rpy="0.0 0.0 0" xyz="0.173 0 -0.04150"/>

🔴 Critical: Hardcoded absolute container paths break non-Docker usage — source/isaaclab_assets/isaaclab_assets/robots/tong_system.py:8

The USD path is hardcoded to /workspace/isaaclab/source/isaaclab_assets/.... This will fail for any user not running inside the specific Docker container. The same issue exists in lift_tong_system_env_cfg.py for the table USD path. Other robot configs in Isaac Lab use ISAACLAB_NUCLEUS_DIR or ISAAC_NUCLEUS_DIR for portable asset resolution.

# Use a path relative to the package or use ISAACLAB_NUCLEUS_DIR
import os
usd_path=os.path.join(os.path.dirname(__file__), "usd", "tong_system", "tong_system.usd"),

🔴 Critical: Docker build context change breaks existing workflows — docker/docker-compose.yaml, docker/Dockerfile.base

Changing the build context from ../ to ../../ and the dockerfile path to IsaacLab/docker/Dockerfile.base assumes IsaacLab lives in a directory literally named IsaacLab alongside a sibling rsl_rl directory. This breaks Docker builds for all existing users. The COPY ./IsaacLab ${ISAACLAB_PATH} and COPY ./rsl_rl ${RSL_RL_PATH} directives enforce this specific directory structure. Additionally, the Dockerfile hardcodes a mv of the system rsl_rl package with a Python 3.11-specific path (/workspace/isaaclab/_isaac_sim/kit/python/lib/python3.11/site-packages/rsl_rl), which will break on other Python versions.

🟡 Warning: OnPolicyRunnerWithDepth import will fail for standard rsl_rl users — scripts/reinforcement_learning/rsl_rl/train.py:85

The import from rsl_rl.runners import OnPolicyRunnerWithDepth is unconditional. Since OnPolicyRunnerWithDepth doesn't exist in the official rsl_rl package (it has 0 search results in leggedrobotics/rsl_rl), this import will raise ImportError for everyone using the standard library. This should either be a conditional/lazy import, or the custom runner class should be contributed upstream first.

# Consider conditional import:
try:
    from rsl_rl.runners import OnPolicyRunnerWithDepth
except ImportError:
    OnPolicyRunnerWithDepth = None

🟡 Warning: URDF <robot name="ur5"> should be "tong_system" — source/isaaclab_assets/isaaclab_assets/robots/urdf_for_usd/tong_system/tong_system.urdf:2

The URDF declares <robot name="ur5"> instead of <robot name="tong_system">. This appears to be a copy-paste artifact from a UR5 URDF template. While this may not cause a runtime error, it produces misleading metadata and confuses anyone inspecting the robot model.

🟡 Warning: normalize_depth doesn't handle negative infinity and doesn't actually normalize — source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/lift/mdp/observations.py:39

The function replaces nan and posinf with 0.0 but does not handle neginf. Depth sensors can produce -inf for invalid readings (e.g., below minimum range). Passing -inf into a neural network will corrupt training. Also, despite the name normalize_depth, no normalization to a bounded range is performed — the raw depth values (potentially up to 1e3 per the camera clipping range) are passed directly to the policy.

def normalize_depth(
    env: ManagerBasedEnv,
    sensor_cfg: SceneEntityCfg,
    data_type: str = "distance_to_image_plane",
) -> torch.Tensor:
    camera = env.scene[sensor_cfg.name]
    depth = camera.data.output[data_type]
    # replace nan and inf to zero
    depth = torch.nan_to_num(depth, nan=0.0, posinf=0.0, neginf=0.0)
    return depth.float()

🟡 Warning: Missing license header and unused import — source/isaaclab_assets/isaaclab_assets/robots/tong_system.py

The file is missing the standard Isaac Lab BSD-3-Clause license header that all other robot config files have. Also, RemotizedPDActuatorCfg is imported but never used.

🔵 Suggestion: Mesh assets should use binary formats or LFS — Multiple .obj/.stl files

This PR adds ~2.85M lines, almost entirely from text-format mesh files (.obj). Some individual files exceed 200K lines (e.g., UpperArm.obj at 206K lines). These should be stored as binary STL (already present as .stl duplicates) or tracked via Git LFS to avoid bloating the repository history. The duplicate .obj + .stl pairs for collision meshes are also redundant — only one format is needed per mesh.

Test Coverage

• Feature PR: No tests included.
• Coverage: No unit or integration tests for the new robot configuration, environment, or the normalize_depth observation function.
• Key gaps: The normalize_depth function should have a basic test verifying NaN/Inf handling. The environment registration should be smoke-tested to verify it can be instantiated.
• Verdict: Coverage is minimal for a feature PR of this scope. At minimum, a smoke test for environment creation would catch configuration errors early.

CI Status

Only the labeler check runs and passes. No code quality, test, or build checks executed — likely because the CI pipeline doesn't trigger on this branch pattern or these file types.

Verdict

COMMENT

This PR introduces several breaking changes to shared infrastructure (Docker build, train.py imports) that would affect all Isaac Lab users, alongside a malformed URDF and hardcoded paths that prevent the new feature from working outside a very specific setup. The robot and task design follow the right patterns, but the implementation needs significant cleanup before it's ready for the main branch. Specifically:

1. Fix the malformed URDF XML (blocking — robot won't load)
2. Replace hardcoded Docker paths with portable alternatives
3. Isolate Docker/rsl_rl changes so they don't break existing users
4. Make the OnPolicyRunnerWithDepth import conditional or contribute it upstream
5. Add the missing license header and remove unused imports
6. Consider Git LFS for the large mesh assets

[isaaclab-review-bot]

🔴 Critical: Hardcoded absolute container path. This path /workspace/isaaclab/source/... only works inside the specific Docker container. All other robot configs in Isaac Lab use ISAACLAB_NUCLEUS_DIR or ISAAC_NUCLEUS_DIR for portable asset resolution. This will cause a FileNotFoundError for anyone running Isaac Lab outside Docker.

Consider using a path relative to this module or a standard Isaac Lab asset directory variable.

[isaaclab-review-bot]

🟡 Missing license header. All files in Isaac Lab require the BSD-3-Clause license header:

# Copyright (c) 2022-2026, The Isaac Lab Project Developers (...)
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause

Also, RemotizedPDActuatorCfg is imported but unused.

[isaaclab-review-bot]

🟡 Warning: Unconditional import of non-standard class. OnPolicyRunnerWithDepth doesn't exist in the official rsl_rl package (leggedrobotics/rsl_rl). This unconditional import will cause ImportError for every user with the standard rsl_rl installation.

Consider a conditional import:

try:
    from rsl_rl.runners import OnPolicyRunnerWithDepth
except ImportError:
    OnPolicyRunnerWithDepth = None

[isaaclab-review-bot]

🟡 Missing neginf handling. Depth sensors commonly produce -inf for invalid readings. torch.nan_to_num with only nan=0.0, posinf=0.0 will pass -inf through to the network, corrupting gradients during training.

Suggested change

	depth = camera.data.output[data_type]
	depth = torch.nan_to_num(depth, nan=0.0, posinf=0.0, neginf=0.0)

Also, the function name normalize_depth is misleading — it doesn't normalize to a bounded range, just sanitizes invalid values.

[isaaclab-review-bot]

🟡 Note: class_name = "OnPolicyRunnerWithDepth" — this runner class is not part of the standard rsl_rl package. Anyone using this config without the custom rsl_rl fork will get a runtime error in train.py. The PPO hyperparameters themselves look reasonable for a manipulation task.