-
Notifications
You must be signed in to change notification settings - Fork 2.2k
SkillGen in Isaac Lab for 2.3 Release #3303
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
…or changes in cuRobo. Added urdf extraction and on-the-fly config creation from nucleus.
README.md
Outdated
@@ -188,6 +188,10 @@ where creativity and technology intersect. Your contributions can make a meaning | |||
The Isaac Lab framework is released under [BSD-3 License](LICENSE). The `isaaclab_mimic` extension and its corresponding standalone scripts are released under [Apache 2.0](LICENSE-mimic). The license files of its dependencies and assets are present in the [`docs/licenses`](docs/licenses) directory. | |||
Note that Isaac Lab requires Isaac Sim, which includes components under proprietary licensing terms. Please see the [Isaac Sim license](https://github.com/isaac-sim/IsaacSim/blob/main/LICENSE) for information on Isaac Sim licensing. |
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.
This should just point to the docs/licenses/dependencies/isaacsim-license.txt license file, which should be updated with the contents of https://github.com/isaac-sim/IsaacSim/blob/main/LICENSE
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.
Corrected! Thanks Gav!
class PreStepSubtaskStartsObservationsRecorder(RecorderTerm): | ||
"""Recorder term that records the subtask start observations in each step.""" | ||
|
||
def record_pre_step(self): |
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.
Missing return type
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.
Done
# Cleanup of motion planners and their visualizers | ||
if motion_planners is not None: | ||
for env_id, planner in motion_planners.items(): | ||
if hasattr(planner, "plan_visualizer") and planner.plan_visualizer is not None: |
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.
Could instead do:
if hasattr(planner, "plan_visualizer") and planner.plan_visualizer is not None: | |
if getattr(planner, "plan_visualizer", None) is not None: |
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.
Added
@@ -30,6 +31,7 @@ def __init__( | |||
eef_pose=None, | |||
object_poses=None, | |||
subtask_term_signals=None, | |||
subtask_start_signals=None, |
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.
Add types?
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.
Type hints for other args were declared in the function docstring as opposed to signature. Just following the same style.
eef_subtask_boundaries[i - 1][1] + prev_max_offset_range | ||
< eef_subtask_boundaries[i][1] + self.subtask_term_offset_ranges[eef_name][i][0] | ||
), ( | ||
"subtask sanity check violation in demo with subtask {} end ind {}, subtask {} max offset {}," |
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.
Use f-strings
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.
Done
|
||
self._monitor_active = True | ||
|
||
def monitor_parent_process(): |
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.
def monitor_parent_process(): | |
def monitor_parent_process() -> None |
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.
Added in the recent commit
self._monitor_thread = threading.Thread(target=monitor_parent_process, daemon=True) | ||
self._monitor_thread.start() | ||
|
||
def _kill_rerun_processes(self): |
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.
def _kill_rerun_processes(self): | |
def _kill_rerun_processes(self) -> None |
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.
Added in the recent commit
def cube_stack_test_env(): | ||
"""Create the environment and motion planner once for the test suite and yield them.""" | ||
random.seed(SEED) | ||
torch.manual_seed(SEED) |
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.
Check that you don't need to also set the numpy
seed
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.
Don't think so, will get back to this later
marker_cfg.markers["frame"].scale = (0.1, 0.1, 0.1) | ||
goal_pose_visualizer = VisualizationMarkers(marker_cfg) | ||
|
||
yield { |
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.
Add a Generator
return type?
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.
Done
"""Test suite for the Curobo motion planner, focusing on obstacle avoidance.""" | ||
|
||
@pytest.fixture(autouse=True) | ||
def setup(self, curobo_test_env): |
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.
Return types here and elsewhere in TestCuroboPlanner
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.
Added in the recent commit
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.
Tested/verified skillgen/prior mimic workflows working as expected. My prior review was completed in IsaacLab-Internal repo and all comments have been addressed in this public PR.
…emoving redundancies
Description
This PR introduces the SkillGen framework to Isaac Lab, integrating GPU motion planning with skill-segmented data generation. It enables efficient, high-quality dataset creation with robust collision handling, visualization, and reproducibility.
Note:
Technical Implementation:
Annotation Framework:
Motion Planning:
Base Motion Planner (Extensible):
source/isaaclab_mimic/isaaclab_mimic/motion_planners/base_motion_planner.py
update_world_and_plan_motion(...)
,get_planned_poses(...)
, etc.CuRobo Planner (GPU-accelerated, collision-aware):
source/isaaclab_mimic/isaaclab_mimic/motion_planners/curobo/test/test_curobo_planner_cube_stack.py
source/isaaclab_mimic/isaaclab_mimic/motion_planners/curobo/test/test_curobo_planner_franka.py
source/isaaclab_mimic/test/test_generate_dataset_skillgen.py
Data Generation Pipeline:
Visualization and Debugging:
Dependencies:
Integration:
This extends the existing mimic pipeline and remains backward compatible. It integrates into the manager-based environment structure and existing observation/action interfaces without breaking current users.
Type of change
Screenshot
SkillGen Data Generation
Bin Cube Stacking Behavior Cloned Policy
Rerun Integration
Motion Planner Tests
Checklist
pre-commit
checks with./isaaclab.sh --format
config/extension.toml
fileCONTRIBUTORS.md
or my name already exists there