-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Gear assembly sim to real #4044
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?
Gear assembly sim to real #4044
Conversation
Greptile Summary
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant TrainingScript
participant Environment
participant GearTypeManager
participant RobotIK
participant PPOAgent
participant RewardManager
User->>TrainingScript: run train.py with task config
TrainingScript->>Environment: create env with UR10e gear assembly config
Environment->>GearTypeManager: initialize RandomizeGearType event
GearTypeManager->>Environment: register as _gear_type_manager
Environment->>Environment: setup scene with robot and 3 gear types
loop Training Episodes
Environment->>GearTypeManager: reset - randomize gear type
GearTypeManager->>Environment: set active gear per env
Environment->>RobotIK: SetRobotToGraspPose event
RobotIK->>RobotIK: run IK to compute grasp pose
RobotIK->>Environment: update robot joint positions
Environment->>Environment: RandomizeGearsAndBasePose event
loop Episode Steps
Environment->>PPOAgent: get observation (joint pos/vel, gear shaft pose)
PPOAgent->>Environment: return action (delta joint positions)
Environment->>Environment: apply action and step simulation
Environment->>RewardManager: compute keypoint distance rewards
RewardManager->>Environment: return reward signal
Environment->>Environment: check terminations (gear dropped, orientation)
end
Environment->>PPOAgent: collect episode data
PPOAgent->>PPOAgent: update policy with PPO
end
TrainingScript->>User: save trained model checkpoint
|
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.
22 files reviewed, 2 comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/deploy/mdp/events.py
Show resolved
Hide resolved
| # TODO: @ashwinvk: Revert to default USD after https://jirasw.nvidia.com/browse/ISIM-4733 is resolved | ||
| usd_path="omniverse://isaac-dev.ov.nvidia.com/Projects/isaac_ros_gear_insertion/ur10e_default_2f85.usd", |
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.
style: temporary USD path override pending ISIM-4733 resolution - ensure this is removed after the issue is fixed
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/deploy/mdp/rewards.py
Outdated
Show resolved
Hide resolved
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/deploy/mdp/rewards.py
Outdated
Show resolved
Hide resolved
...tasks/manager_based/manipulation/deploy/gear_assembly/config/ur_10e/agents/rsl_rl_ppo_cfg.py
Outdated
Show resolved
Hide resolved
| noise_std_type: Literal["scalar", "log"] = "scalar" | ||
| """The type of noise standard deviation for the policy. Default is scalar.""" | ||
|
|
||
| state_dependent_std: bool = False |
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 might be a different PR.
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 param in used in the rsl rl config for the sim to real env. Are you saying that it should be seperated into a new PR?
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.
yes a different PR to introduce this argument in rl_cfg.py
...tasks/manager_based/manipulation/deploy/gear_assembly/config/ur_10e/agents/rsl_rl_ppo_cfg.py
Outdated
Show resolved
Hide resolved
| prim_path="{ENV_REGEX_NS}/FactoryGearBase", | ||
| # TODO: change to common isaac sim directory | ||
| spawn=sim_utils.UsdFileCfg( | ||
| usd_path="omniverse://isaac-dev.ov.nvidia.com/Isaac/Props/Factory/gear_assets/factory_gear_base/factory_gear_base.usd", |
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.
please use import NUCLEUS Directory import instead write raw url
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.
Waiting on the sim team to upload these assets to the AWS server and I will replace it.
...asks/isaaclab_tasks/manager_based/manipulation/deploy/gear_assembly/gear_assembly_env_cfg.py
Outdated
Show resolved
Hide resolved
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.
please capture a high quality image with robot base. : )))
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. Does this one work?
kellyguo11
left a comment
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 we try to avoid having large .gif files in the repo directly? we can upload them to the server if needed and referenced from docs.
| prim_path="{ENV_REGEX_NS}/FactoryGearSmall", | ||
| # TODO: change to common isaac sim directory | ||
| spawn=sim_utils.UsdFileCfg( | ||
| usd_path="omniverse://isaac-dev.ov.nvidia.com/Isaac/Props/Factory/gear_assets/factory_gear_small/factory_gear_small.usd", |
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.
we shouldn't use this path directly in code being merged to main, users will not have access to these. are the assets available on the S3 bucket?
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.
yup, waiting on the sim team to uplaod assets and I will update before this PR is merged
| mode="reset", | ||
| params={ | ||
| "gear_types": ["gear_small", "gear_medium", "gear_large"] | ||
| # "gear_types": ["gear_small"] |
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.
will remove
| mode="startup", | ||
| params={ | ||
| "asset_cfg": SceneEntityCfg("robot", body_names=".*finger"), | ||
| "static_friction_range": (1000.0, 1000.0), |
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.
will test and update to 1.5 instead of 1000.0
|
Thanks for the edit and contribution : )), I'd like to ask a high level questions why not put this PR in deploy folder we created for the reach eariler? Are we planing to add peg insert and nut thread as well? if that's the intention, it might be more benefitial to work a general structure with all of them, right now mdp seems just tailored to gearmesh. |
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.
having the asset in the air is a bit odd, can we add a desk to it?
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 base is a fixed asset whose pose is randomzied in the all 6 degrees of freedom. I think adding a table would cause collision issues or visual penetration. And it might make the user think we're actually simulating the base->table physics interactions.
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.
Do you think I should I should add a note in the docs about this?
rl-video-step-137600.mp4
Description
This PR introduces a new Gear Assembly manipulation task for sim-to-real training with the UR10e robot arm. This environment enables training policies for precise gear insertion tasks using reinforcement learning, with comprehensive sim-to-real transfer capabilities.
Summary of Changes
New Features
Gear Assembly Environment: Complete environment implementation for gear insertion tasks
gear_assembly_env_cfg.py)joint_pos_env_cfg.py)rsl_rl_ppo_cfg.py)MDP Components: Task-specific observation, reward, termination, and event functions
mdp/events.py: Randomization and reset events for robust trainingmdp/observations.py: State observation functionsmdp/rewards.py: Reward shaping for gear insertionmdp/terminations.py: Episode termination conditionsNoise Models: Enhanced noise simulation for domain randomization
noise_model.py,noise_cfg.py)Documentation
Core Enhancements
train.pywith additional logging and configuration optionsuniversal_robots.pywith gear assembly specific parametersisaaclab/envs/mdp/rewards.pyrl_cfg.py,setup.py)Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists thereUsage Example