Skip to content

Conversation

@yuecideng
Copy link
Contributor

@yuecideng yuecideng commented Dec 27, 2025

Description

  • Rename original rendering module into visual in the randomization module
  • Add a simple mass randomizer functor

Type of change

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

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • I have run the black . command to format the code base.
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • Dependencies have been updated, if applicable.

@yuecideng yuecideng added physics Things related to physics gym robot learning env and its related features labels Dec 27, 2025
@yuecideng yuecideng requested a review from Copilot December 28, 2025 09:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds basic physics randomization capabilities to the framework by introducing a mass randomization functor, and renames the rendering module to visual for better clarity.

Key Changes

  • Added a new physics.py module with a randomize_rigid_object_mass function for randomizing rigid object masses
  • Renamed the rendering module to visual across the codebase for more accurate terminology
  • Added set_mass and get_mass methods to the RigidObject class for mass manipulation

Reviewed changes

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

Show a summary per file
File Description
embodichain/lab/gym/envs/managers/randomization/physics.py New physics randomization module with mass randomization function
embodichain/lab/sim/objects/rigid_object.py Added mass getter and setter methods for rigid objects
tests/sim/objects/test_rigid_object.py Added test for mass getter/setter methods and configured initial mass
embodichain/utils/math.py Added device parameter to sample_uniform function with default value
embodichain/lab/gym/envs/managers/randomization/visual.py Removed unused static method gen_random_base_color_texture
embodichain/lab/gym/envs/managers/randomization/spatial.py Updated sample_uniform calls to include device parameter
embodichain/lab/sim/objects/articulation.py Removed unnecessary torch.cuda.synchronize calls for performance
embodichain/lab/sim/material.py Added debug sleep statement (should be removed)
embodichain/lab/gym/envs/managers/randomization/init.py Updated imports to reflect module rename from rendering to visual, added physics import
embodichain/lab/gym/envs/embodied_env.py Updated module path reference from rendering to visual
scripts/tutorials/gym/modular_env.py Added example usage of the new mass randomization function
docs/source/api_reference/embodichain/embodichain.lab.gym.envs.managers.rst Updated documentation to reflect module rename and added physics section
docs/source/quick_start/install.md Simplified and reformatted installation documentation
pyproject.toml Pinned dexsim_engine version to 0.3.8

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +534 to +536
for i, env_idx in enumerate(local_env_ids):
mass = self._entities[env_idx].get_physical_body().get_mass()
masses.append(mass)
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

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

The loop variable i is declared but never used. Since the loop only needs env_idx, consider removing the unused variable by using for env_idx in enumerate(local_env_ids) or using _ as a placeholder: for _, env_idx in enumerate(local_env_ids).

Copilot uses AI. Check for mistakes.
num_instance = len(env_ids)

sampled_masses = sample_uniform(
lower=mass_range[0], upper=mass_range[1], size=(num_instance,)
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

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

The device parameter is missing in the sample_uniform call. While a default device of "cpu" is now available in the function signature, this could lead to a device mismatch when env.device is CUDA. The sampled masses will be on CPU, but then need to be moved to the correct device later. Consider passing device=env.device to ensure the sampled masses are created on the correct device from the start.

Suggested change
lower=mass_range[0], upper=mass_range[1], size=(num_instance,)
lower=mass_range[0],
upper=mass_range[1],
size=(num_instance,),
device=env.device,

Copilot uses AI. Check for mistakes.
Comment on lines +20 to +26
from typing import TYPE_CHECKING, Literal, Union, List

from embodichain.lab.sim.objects import RigidObject, Robot
from embodichain.lab.sim.cfg import RigidObjectCfg
from embodichain.lab.gym.envs.managers.cfg import SceneEntityCfg
from embodichain.utils.math import sample_uniform, matrix_from_euler
from embodichain.utils import logger
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

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

There are unused imports in this file. Robot, RigidObjectCfg, Literal, matrix_from_euler, and logger are imported but never used in the code. These should be removed to keep the imports clean.

Suggested change
from typing import TYPE_CHECKING, Literal, Union, List
from embodichain.lab.sim.objects import RigidObject, Robot
from embodichain.lab.sim.cfg import RigidObjectCfg
from embodichain.lab.gym.envs.managers.cfg import SceneEntityCfg
from embodichain.utils.math import sample_uniform, matrix_from_euler
from embodichain.utils import logger
from typing import TYPE_CHECKING, Union, List
from embodichain.lab.sim.objects import RigidObject
from embodichain.lab.gym.envs.managers.cfg import SceneEntityCfg
from embodichain.utils.math import sample_uniform

Copilot uses AI. Check for mistakes.
Comment on lines +258 to +260
import time

time.sleep(0.6)
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

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

Debug code has been left in production. This time.sleep(0.6) call appears to be temporary debugging code that should be removed before merging. It will cause unnecessary delays in texture setting operations.

Suggested change
import time
time.sleep(0.6)

Copilot uses AI. Check for mistakes.
Comment on lines +17 to 20
from .physics import *
from .visual import *
from .spatial import *

Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module embodichain.lab.gym.envs.managers.randomization.physics does not define 'all'.

Suggested change
from .physics import *
from .visual import *
from .spatial import *
from . import physics as _physics
from . import visual as _visual
from . import spatial as _spatial
__all__ = []
for _mod in (_physics, _visual, _spatial):
_names = getattr(_mod, "__all__", [n for n in dir(_mod) if not n.startswith("_")])
__all__.extend(_names)
for _name in _names:
globals()[_name] = getattr(_mod, _name)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gym robot learning env and its related features physics Things related to physics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants