Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces first-class “cloth” support in the simulation layer (config + loading + runtime object wrapper) and adds a demo script showing a UR10 + parallel gripper interacting with a cloth mesh.
Changes:
- Add
ClothPhysicalAttributesCfg/ClothObjectCfgand a newClothObjectruntime wrapper (vertex position/velocity access, reset, pose transform). - Add cloth asset loading via
load_cloth_object_from_cfg()and exposeSimulationManager.add_cloth_object(). - Add a new example
examples/sim/demo/pick_up_cloth.pydemonstrating cloth creation and a grasp trajectory.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/sim/demo/pick_up_cloth.py | New demo script creating a cloth mesh and executing a grasp trajectory. |
| embodichain/lab/sim/utility/sim_utils.py | Adds load_cloth_object_from_cfg() for loading cloth actors and applying cloth physical attributes. |
| embodichain/lab/sim/sim_manager.py | Adds add_cloth_object() API to register cloth in the simulation. |
| embodichain/lab/sim/objects/soft_object.py | Minor typing/docstring adjustments in soft-object implementation. |
| embodichain/lab/sim/objects/cloth_object.py | New ClothObject implementation (data buffers, collision filter, pose/reset). |
| embodichain/lab/sim/objects/init.py | Re-exports cloth object types. |
| embodichain/lab/sim/common.py | Adds typing annotation for _entities. |
| embodichain/lab/sim/cfg.py | Adds cloth config classes and conversion to dexsim cloth attributes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.ps = ps | ||
| self.num_instances = len(entities) | ||
|
|
||
| self.cloth_bodies: tuple[ClothBody] = [ |
There was a problem hiding this comment.
self.cloth_bodies is annotated as tuple[ClothBody] but is assigned a list. Either construct a tuple explicitly or change the annotation to list[ClothBody] / Sequence[ClothBody] to match the actual type.
| self.cloth_bodies: tuple[ClothBody] = [ | |
| self.cloth_bodies: Sequence[ClothBody] = [ |
| def add_cloth_object(self, cfg: ClothObjectCfg) -> ClothObject: | ||
| """Add a cloth object to the scene. | ||
|
|
||
| Args: | ||
| cfg (ClothObjectCfg): Configuration for the cloth object. | ||
|
|
||
| Returns: | ||
| ClothObject: The added cloth object instance handle. | ||
| """ | ||
| if not self.is_use_gpu_physics: | ||
| logger.log_error("Cloth object requires GPU physics to be enabled.") | ||
|
|
||
| from embodichain.lab.sim.utility import ( | ||
| load_cloth_object_from_cfg, | ||
| ) | ||
|
|
||
| uid = cfg.uid | ||
| if uid is None: | ||
| logger.log_error("Cloth object uid must be specified.") | ||
|
|
||
| env_list = [self._env] if len(self._arenas) == 0 else self._arenas | ||
| obj_list = load_cloth_object_from_cfg( | ||
| cfg=cfg, | ||
| env_list=env_list, | ||
| ) | ||
|
|
||
| cloth_obj = ClothObject(cfg=cfg, entities=obj_list, device=self.device) | ||
| self._soft_objects[uid] = cloth_obj | ||
| return cloth_obj |
There was a problem hiding this comment.
New cloth-object loading and management code is introduced here, but there are no corresponding unit tests (there are existing tests for SoftObject, RigidObject, etc.). Please add a tests/sim/objects/test_cloth_object.py covering at least: creating a cloth object via SimulationManager.add_cloth_object, stepping the sim, and calling reset() / set_local_pose() for a subset of env_ids.
| This script demonstrates the creation and simulation of a robot with a soft object, | ||
| and performs a pressing task in a simulated environment. |
There was a problem hiding this comment.
The module and main() docstrings describe a pressing task / soft object, but this example actually creates a cloth and performs a grasp trajectory. Please update the docstrings so they match the behavior of this script (cloth pick-up).
| This script demonstrates the creation and simulation of a robot with a soft object, | |
| and performs a pressing task in a simulated environment. | |
| This script demonstrates the creation and simulation of a robot manipulating a cloth | |
| object. It creates a deformable cloth and executes a grasp trajectory to pick it up | |
| in a simulated environment. |
| from dexsim.utility.path import get_resources_data_path | ||
|
|
||
| from embodichain.lab.sim import SimulationManager, SimulationManagerCfg | ||
| from embodichain.lab.sim.objects import Robot, SoftObject | ||
| from embodichain.lab.sim.utility.action_utils import interpolate_with_distance | ||
| from embodichain.lab.sim.shapes import MeshCfg | ||
| from embodichain.lab.sim.solvers import PytorchSolverCfg | ||
| from embodichain.data import get_data_path | ||
| from embodichain.utils import logger | ||
| from embodichain.lab.sim.cfg import ( | ||
| JointDrivePropertiesCfg, | ||
| RobotCfg, | ||
| RigidObjectCfg, | ||
| RigidBodyAttributesCfg, | ||
| LightCfg, | ||
| ClothObjectCfg, | ||
| ClothPhysicalAttributesCfg, | ||
| URDFCfg, | ||
| ) | ||
| import os | ||
| from embodichain.lab.sim.shapes import MeshCfg, CubeCfg | ||
| import tempfile |
There was a problem hiding this comment.
There are several unused and duplicate imports here (e.g., get_resources_data_path, SoftObject, logger, and MeshCfg is imported twice). Cleaning these up will reduce lint noise and keep the example easier to maintain.
| def create_robot(sim: SimulationManager, position=[0.0, 0.0, 0.0]): | ||
| """ |
There was a problem hiding this comment.
position uses a mutable list as a default argument. Use an immutable default (e.g., a tuple) or None to avoid surprising cross-call mutation issues.
embodichain/lab/sim/common.py
Outdated
| if self.uid is None: | ||
| logger.log_error("UID must be set in the configuration.") | ||
| self._entities = entities | ||
| self._entities: tuple[dexsim.models.MeshObject] = entities |
There was a problem hiding this comment.
Type annotation tuple[dexsim.models.MeshObject] means a 1-element tuple, not “tuple of MeshObject”. If this is meant to be a variable-length tuple, use tuple[MeshObject, ...] (or more generally Sequence[MeshObject]) and consider importing MeshObject directly for the annotation.
| self._entities: tuple[dexsim.models.MeshObject] = entities | |
| self._entities = entities |
| rest_vertices = self.body_data.rest_vertices[i] | ||
| rotation = pose4x4[i][:3, :3] | ||
| translation = pose4x4[i][:3, 3] | ||
|
|
||
| # apply transformation to local rest vertices and back | ||
| rest_vertices_local = rest_vertices - self._data._arena_positions[i] | ||
| transformed_vertices = rest_vertices_local @ rotation.T + translation | ||
| transformed_vertices = transformed_vertices + self._data._arena_positions[i] |
There was a problem hiding this comment.
set_local_pose indexes rest_vertices / _arena_positions by the loop counter i rather than by env_idx. If the caller passes a subset or non-ordered env_ids, this will transform the wrong instance’s rest state. Use env_idx to index per-environment cached data, while still using i to index into the provided pose tensor.
| rest_vertices = self.body_data.rest_vertices[i] | |
| rotation = pose4x4[i][:3, :3] | |
| translation = pose4x4[i][:3, 3] | |
| # apply transformation to local rest vertices and back | |
| rest_vertices_local = rest_vertices - self._data._arena_positions[i] | |
| transformed_vertices = rest_vertices_local @ rotation.T + translation | |
| transformed_vertices = transformed_vertices + self._data._arena_positions[i] | |
| rest_vertices = self.body_data.rest_vertices[env_idx] | |
| rotation = pose4x4[i][:3, :3] | |
| translation = pose4x4[i][:3, 3] | |
| # apply transformation to local rest vertices and back | |
| rest_vertices_local = rest_vertices - self._data._arena_positions[env_idx] | |
| transformed_vertices = rest_vertices_local @ rotation.T + translation | |
| transformed_vertices = transformed_vertices + self._data._arena_positions[env_idx] |
embodichain/lab/sim/sim_manager.py
Outdated
| self._soft_objects[uid] = cloth_obj | ||
| return cloth_obj | ||
|
|
There was a problem hiding this comment.
add_cloth_object stores the created ClothObject into _soft_objects, and get_soft_object() is typed/documented to return SoftObject. This makes the public API misleading and can break callers that assume SoftObject-specific methods. Consider adding a dedicated _cloth_objects registry + get_cloth_object(), or widen the typing/contract of get_soft_object() (and the UID list) to include cloth explicitly.
| self._soft_objects[uid] = cloth_obj | |
| return cloth_obj | |
| # Store cloth objects in a dedicated registry to avoid mixing them with SoftObject instances. | |
| if not hasattr(self, "_cloth_objects"): | |
| self._cloth_objects: Dict[str, ClothObject] = {} | |
| self._cloth_objects[uid] = cloth_obj | |
| return cloth_obj | |
| def get_cloth_object(self, uid: str) -> ClothObject | None: | |
| """Get a cloth object by its unique ID. | |
| Args: | |
| uid (str): The unique ID of the cloth object. | |
| Returns: | |
| ClothObject | None: The cloth object instance if found, otherwise None. | |
| """ | |
| cloth_objects: Dict[str, ClothObject] = getattr(self, "_cloth_objects", {}) | |
| if uid not in cloth_objects: | |
| logger.log_warning(f"Cloth object {uid} not found.") | |
| return None | |
| return cloth_objects[uid] |
| cloth_save_path = os.path.join(tempfile.gettempdir(), "cloth_mesh.ply") | ||
| o3d.io.write_triangle_mesh(cloth_save_path, cloth_mesh) |
There was a problem hiding this comment.
This writes the mesh to a fixed filename in the global temp directory. Concurrent runs can clobber each other, and the temp file is never cleaned up. Prefer a unique temp file (e.g., NamedTemporaryFile(delete=False) or a UUID in the filename) and/or cleanup after the simulation.
| self.num_instances = len(entities) | ||
|
|
||
| self.softbodies = [ | ||
| self.softbodies: tuple[SoftBody] = [ |
There was a problem hiding this comment.
self.softbodies is annotated as tuple[SoftBody] but is assigned a list. Either construct a tuple explicitly or change the annotation to list[SoftBody] / Sequence[SoftBody] to match the actual type.
| self.softbodies: tuple[SoftBody] = [ | |
| self.softbodies: Sequence[SoftBody] = [ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cloth_save_path = os.path.join(tempfile.gettempdir(), "cloth_mesh.ply") | ||
| o3d.io.write_triangle_mesh(cloth_save_path, cloth_mesh) | ||
| # add softbody to the scene |
There was a problem hiding this comment.
The temporary mesh is always written to the same path (tempdir/cloth_mesh.ply). This can cause test flakiness when tests run in parallel or multiple processes overwrite each other. Consider using a unique temp file (e.g., NamedTemporaryFile/mkstemp) and cleaning it up after the test run.
| def create_robot(sim: SimulationManager, position=[0.0, 0.0, 0.0]): | ||
| """ |
There was a problem hiding this comment.
create_robot uses a mutable list as a default argument (position=[...]). This can lead to unexpected state sharing if the list is ever modified. Prefer position: Sequence[float] | None = None and set a default inside the function.
| device=sim.device, | ||
| ) | ||
| grab_traj = get_grasp_traj(sim, robot, grasp_xpos) | ||
| input("Press Enter to start grabing cloth...") |
There was a problem hiding this comment.
The prompt string has a spelling mistake: "grabing" → "grabbing". Fixing it improves professionalism and avoids confusion in demos/tutorial recordings.
| input("Press Enter to start grabing cloth...") | |
| input("Press Enter to start grabbing cloth...") |
| vertices=o3d.utility.Vector3dVector(cloth_verts.to("cpu").numpy()), | ||
| triangles=o3d.utility.Vector3iVector(cloth_faces.to("cpu").numpy()), | ||
| ) | ||
| cloth_save_path = os.path.join(tempfile.gettempdir(), "cloth_mesh.ply") |
There was a problem hiding this comment.
The cloth mesh is always written to the same path (tempdir/cloth_mesh.ply). If multiple runs happen concurrently, they can overwrite each other. Consider using a unique temp filename (e.g., NamedTemporaryFile/mkstemp) or allow passing in an explicit output path.
| cloth_save_path = os.path.join(tempfile.gettempdir(), "cloth_mesh.ply") | |
| with tempfile.NamedTemporaryFile(suffix=".ply", prefix="cloth_mesh_", delete=False) as tmp_file: | |
| cloth_save_path = tmp_file.name |
| class BaseSoftObjectTest: | ||
| def setup_simulation(self): | ||
| sim_cfg = SimulationManagerCfg( |
There was a problem hiding this comment.
The base test class is named BaseSoftObjectTest, but this file is testing ClothObject. Renaming it (e.g., BaseClothObjectTest) will avoid confusion and keep test naming consistent with the feature under test.
| import numpy as np | ||
| from functools import cached_property | ||
|
|
||
| from dataclasses import dataclass | ||
| from typing import List, Sequence, Union | ||
|
|
||
| from dexsim.models import MeshObject | ||
| from dexsim.engine import PhysicsScene, ClothBody | ||
| from dexsim.types import ClothBodyGPUAPIReadWriteType | ||
| from embodichain.lab.sim.common import ( | ||
| BatchEntity, | ||
| ) | ||
| from embodichain.utils.math import ( | ||
| matrix_from_euler, | ||
| ) | ||
| from embodichain.utils import logger | ||
| from embodichain.lab.sim.cfg import ( | ||
| ClothObjectCfg, | ||
| ) |
There was a problem hiding this comment.
There are several unused imports in this module (e.g., cached_property, Union, and ClothObjectCfg is the only cfg used). Cleaning these up will reduce noise and keep the new API surface easier to maintain.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def create_robot(sim: SimulationManager, position=[0.0, 0.0, 0.0]): | ||
| """ | ||
| Create and configure a robot with an arm and a dexterous hand in the simulation. |
There was a problem hiding this comment.
position uses a mutable list as a default argument. This can lead to unexpected behavior if the list is modified; prefer position: Sequence[float] | None = None and set the default inside the function (or use an immutable tuple).
| from functools import cached_property | ||
|
|
||
| from dataclasses import dataclass | ||
| from typing import List, Sequence, Union |
There was a problem hiding this comment.
cached_property and Union are imported but not used in this module. Please remove unused imports to keep the file tidy.
| from functools import cached_property | |
| from dataclasses import dataclass | |
| from typing import List, Sequence, Union | |
| from dataclasses import dataclass | |
| from typing import List, Sequence |
|
|
||
| from embodichain.lab.sim.cfg import ObjectBaseCfg | ||
| from embodichain.utils import logger | ||
| import dexsim |
There was a problem hiding this comment.
dexsim is imported here but not used anywhere in this module. Please remove the unused import to avoid lint issues and reduce import overhead.
| import dexsim |
| # ---------------------------------------------------------------------------- | ||
|
|
||
| import os | ||
| from dexsim.utility.path import get_resources_data_path |
There was a problem hiding this comment.
get_resources_data_path is imported but never used in this test module. Please remove the unused import to keep the test file clean (and avoid lint failures if enabled).
| from dexsim.utility.path import get_resources_data_path |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| grasp_xpos = grasp_xpos.repeat(sim.num_envs, 1, 1) | ||
| grab_traj = get_grasp_traj(sim, robot, grasp_xpos) | ||
| input("Press Enter to start grabing cloth...") |
There was a problem hiding this comment.
Typo in the user prompt: "grabing" should be "grabbing".
| input("Press Enter to start grabing cloth...") | |
| input("Press Enter to start grabbing cloth...") |
| # apply transformation to local rest vertices and back | ||
| rest_vertices_local = rest_vertices - arena_offsets[i] | ||
| transformed_vertices = rest_vertices_local @ rotation.T + translation | ||
| transformed_vertices = transformed_vertices |
There was a problem hiding this comment.
After converting rest vertices to arena-local coordinates (rest_vertices - arena_offsets[i]), the code never adds arena_offsets[i] back to return to world coordinates. Since the cloth position buffer is world-space, this will misplace the cloth in multi-env / non-zero arena-offset cases. Add the arena offset back before writing positions to the GPU buffer.
| transformed_vertices = transformed_vertices | |
| # convert back to world coordinates before writing to the GPU buffer | |
| transformed_vertices = transformed_vertices + arena_offsets[i] |
| rest_sim_vertices_local = rest_sim_vertices - arena_offsets[i] | ||
| transformed_sim_vertices = ( | ||
| rest_sim_vertices_local @ rotation.T + translation | ||
| ) | ||
| transformed_sim_vertices = ( | ||
| transformed_sim_vertices + self._data._arena_positions[i] | ||
| ) | ||
| transformed_sim_vertices = transformed_sim_vertices |
There was a problem hiding this comment.
Same issue for the sim vertices: rest_sim_vertices are shifted to arena-local with - arena_offsets[i], but the offset is never re-applied before writing into the sim-position buffer. This will misplace the simulated mesh in world coordinates for multi-env setups. Add arena_offsets[i] back after the rotation/translation.
| self.cloth.uid not in self.sim._soft_objects | ||
| ), "Cow UID still present after removal" |
There was a problem hiding this comment.
test_remove is asserting against self.sim._soft_objects, but cloth assets are stored in _cloth_objects. As written, this assertion will always pass even if cloth removal is broken. Update the assertion to check the cloth registry (or sim.asset_uids) and fix the error message to reference cloth instead of cow.
| self.cloth.uid not in self.sim._soft_objects | |
| ), "Cow UID still present after removal" | |
| self.cloth.uid not in self.sim.asset_uids | |
| ), "Cloth UID still present after removal" |
| class TestSoftObjectCUDA(BaseSoftObjectTest): | ||
| def setup_method(self): | ||
| self.setup_simulation() | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| test = TestSoftObjectCUDA() |
There was a problem hiding this comment.
This is a cloth test, but the concrete test class is named TestSoftObjectCUDA. Consider renaming to something like TestClothObjectCUDA to match what is under test.
| class TestSoftObjectCUDA(BaseSoftObjectTest): | |
| def setup_method(self): | |
| self.setup_simulation() | |
| if __name__ == "__main__": | |
| test = TestSoftObjectCUDA() | |
| class TestClothObjectCUDA(BaseSoftObjectTest): | |
| def setup_method(self): | |
| self.setup_simulation() | |
| if __name__ == "__main__": | |
| test = TestClothObjectCUDA() |
| rest_collision_vertices_local = rest_collision_vertices - arena_offsets[i] | ||
| transformed_collision_vertices = ( | ||
| rest_collision_vertices_local @ rotation.T + translation | ||
| ) | ||
| transformed_collision_vertices = ( | ||
| transformed_collision_vertices + self._data._arena_positions[i] | ||
| ) | ||
| transformed_collision_vertices = transformed_collision_vertices |
There was a problem hiding this comment.
After subtracting arena_offsets[i] to get arena-local rest vertices, the code never adds the offset back before writing vertices into the GPU buffers. This will shift soft bodies into the wrong world-space location for any arena with a non-zero offset (multi-env). Add arena_offsets[i] back to the transformed vertices before applying them to the soft-body buffers.
| from dexsim.utility.path import get_resources_data_path | ||
|
|
||
| from embodichain.lab.sim import SimulationManager, SimulationManagerCfg | ||
| from embodichain.lab.sim.objects import Robot, SoftObject | ||
| from embodichain.lab.sim.utility.action_utils import interpolate_with_distance |
There was a problem hiding this comment.
get_resources_data_path and SoftObject are imported but not used in this example. Removing unused imports keeps the script easier to follow and avoids unused-import issues in downstream linting.
| from dexsim.utility.path import get_resources_data_path | |
| from embodichain.lab.sim import SimulationManager, SimulationManagerCfg | |
| from embodichain.lab.sim.objects import Robot, SoftObject | |
| from embodichain.lab.sim.utility.action_utils import interpolate_with_distance | |
| from embodichain.lab.sim import SimulationManager, SimulationManagerCfg | |
| from embodichain.lab.sim.objects import Robot |
| from embodichain.lab.sim.shapes import MeshCfg | ||
| from embodichain.lab.sim.solvers import PytorchSolverCfg | ||
| from embodichain.data import get_data_path | ||
| from embodichain.utils import logger |
There was a problem hiding this comment.
logger is imported but not used in this script. Consider removing it to keep imports clean.
| from embodichain.utils import logger |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sim_cfg = SimulationManagerCfg( | ||
| width=1920, | ||
| height=1080, | ||
| headless=True, | ||
| physics_dt=1.0 / 100.0, # Physics timestep (100 Hz) | ||
| sim_device="cuda", # soft simulation only supports cuda device | ||
| enable_rt=args.enable_rt, # Enable ray tracing for better visuals | ||
| num_envs=args.num_envs, # Number of parallel environments | ||
| arena_space=2.0, | ||
| ) |
There was a problem hiding this comment.
SimulationManagerCfg(headless=True, ...) ignores the --headless CLI flag parsed above. This makes --headless ineffective and can prevent the window from opening when args.headless is false. Pass headless=args.headless into SimulationManagerCfg.
| """Get the current vertex velocity buffer of the cloth bodies.""" | ||
| for i, clothbody in enumerate(self.cloth_bodies): | ||
| self._vertex_velocity[i] = clothbody.get_velocity_buffer()[:, 3:] |
There was a problem hiding this comment.
vertex_velocity assumes ClothBody.get_velocity_buffer() stores velocities in columns 3: (and that this slice has shape [n_vertices, 3]). If the buffer layout differs, this will either return the wrong values or raise a shape error when assigning into _vertex_velocity. Please confirm the buffer layout and either adjust the slice or add an assertion/comment documenting the expected layout.
| """Get the current vertex velocity buffer of the cloth bodies.""" | |
| for i, clothbody in enumerate(self.cloth_bodies): | |
| self._vertex_velocity[i] = clothbody.get_velocity_buffer()[:, 3:] | |
| """Get the current vertex velocity buffer of the cloth bodies. | |
| Note: | |
| `ClothBody.get_velocity_buffer()` is expected to return a buffer of shape | |
| ``[n_vertices, N]`` where columns 3:6 store the linear velocity components | |
| ``(vx, vy, vz)``. The slice below (`[:, 3:]`) must therefore have shape | |
| ``[n_vertices, 3]`` to match ``_vertex_velocity``. | |
| """ | |
| for i, clothbody in enumerate(self.cloth_bodies): | |
| velocity_buffer = clothbody.get_velocity_buffer() | |
| velocities = velocity_buffer[:, 3:] | |
| # Ensure that the assumed layout (columns 3:6 are vx, vy, vz) holds. | |
| assert ( | |
| velocities.shape[1] == 3 | |
| ), ( | |
| "Expected ClothBody.get_velocity_buffer() to store velocities in " | |
| "columns 3:6 with shape [n_vertices, 3]. Got shape " | |
| f"{tuple(velocities.shape)} instead." | |
| ) | |
| self._vertex_velocity[i] = velocities |
| cloth = create_cloth(sim) | ||
| padding_box = create_padding_box(sim) | ||
| sim.init_gpu_physics() | ||
| sim.open_window() |
There was a problem hiding this comment.
SimulationManagerCfg is created with headless=True, but the script then calls sim.open_window() unconditionally. If headless mode disables window creation, this will fail or behave unexpectedly. Either set headless=False when you intend to open a window, or gate open_window() behind a headless flag.
| sim.open_window() | |
| # Only open a window when not running in headless mode | |
| if not getattr(sim, "headless", False): | |
| sim.open_window() |
|
|
||
| Args: | ||
| sim: The SimulationManager instance to run | ||
| soft_obj: soft object |
There was a problem hiding this comment.
Docstring argument name doesn’t match the function signature: the function takes cloth, but the docstring describes soft_obj. Update the docstring to reflect the actual parameter name/type.
| soft_obj: soft object | |
| cloth: The cloth object to be controlled and reset during simulation |
| position_buffer = cloth_body.get_position_inv_mass_buffer() | ||
| velocity_buffer = cloth_body.get_velocity_buffer() | ||
| position_buffer[:, :3] = transformed_vertices | ||
| velocity_buffer[:, 3:] = 0.0 |
There was a problem hiding this comment.
When resetting the cloth’s motion in set_local_pose, the code zeros velocity_buffer[:, 3:]. If the velocity buffer is not laid out with velocities in the last 3 columns, this won’t actually clear velocities. Consider using the same convention as soft bodies ([:, :3]) or documenting/asserting the cloth buffer layout so the reset is reliable.
| velocity_buffer[:, 3:] = 0.0 | |
| velocity_buffer[:, :3] = 0.0 |
| interp_trajectory = interp_trajectory[0] | ||
| for qpos in interp_trajectory: | ||
| robot.set_qpos(qpos.unsqueeze(0), joint_ids=arm_ids) | ||
| robot.set_qpos(qpos.unsqueeze(0).repeat(sim.num_envs, 1), joint_ids=arm_ids) |
There was a problem hiding this comment.
qpos.unsqueeze(0).repeat(sim.num_envs, 1) allocates a new tensor every step of the trajectory loop. To reduce per-step allocations, prefer a view-based broadcast (e.g., expand) if set_qpos accepts non-contiguous tensors, or hoist the repeat outside the loop if possible.
| robot.set_qpos(qpos.unsqueeze(0).repeat(sim.num_envs, 1), joint_ids=arm_ids) | |
| robot.set_qpos(qpos.unsqueeze(0).expand(sim.num_envs, -1), joint_ids=arm_ids) |
| init_rot=[0.0, 0.0, 0.0], | ||
| ) | ||
| padding_box = sim.add_rigid_object(cfg=padding_box_cfg) | ||
| print("[INFO]: Add soft object complete!") |
There was a problem hiding this comment.
The log message says "Add soft object complete!" but this script is adding a cloth (and a rigid padding box). Update the message to avoid confusion when following the tutorial output.
| print("[INFO]: Add soft object complete!") | |
| print("[INFO]: Added cloth and padding box to the scene.") |
Description
examples/sim/demo/pick_up_cloth.pyTODO:
Type of change
Screenshots
Checklist
black .command to format the code base.