-
Notifications
You must be signed in to change notification settings - Fork 1
Add basic physcis randomization functor #54
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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.pymodule with arandomize_rigid_object_massfunction for randomizing rigid object masses - Renamed the
renderingmodule tovisualacross the codebase for more accurate terminology - Added
set_massandget_massmethods to theRigidObjectclass 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.
| for i, env_idx in enumerate(local_env_ids): | ||
| mass = self._entities[env_idx].get_physical_body().get_mass() | ||
| masses.append(mass) |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
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).
| num_instance = len(env_ids) | ||
|
|
||
| sampled_masses = sample_uniform( | ||
| lower=mass_range[0], upper=mass_range[1], size=(num_instance,) |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
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.
| 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, |
| 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 |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
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.
| 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 |
| import time | ||
|
|
||
| time.sleep(0.6) |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
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.
| import time | |
| time.sleep(0.6) |
| from .physics import * | ||
| from .visual import * | ||
| from .spatial import * | ||
|
|
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
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'.
| 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) |
Description
renderingmodule intovisualin the randomization moduleType of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
black .command to format the code base.