Add PushBall RL env #96
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a new reinforcement learning environment for pushing a soccer ball to a goal using a UR10 robot arm. The implementation follows the existing pattern from the PushCubeRL environment but introduces several inconsistencies and configuration issues.
Changes:
- Added
PushBallGateEnvclass implementing the ball pushing task with goal checking and truncation logic - Added training configuration with PPO algorithm settings
- Added gym configuration with robot setup, ball physics, events, observations, and reward functions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| embodichain/lab/gym/envs/tasks/rl/push_ball.py | New RL environment class for ball pushing task with success/failure logic |
| configs/agents/rl/push_ball/train_config.json | Training configuration with PPO hyperparameters and network architecture |
| configs/agents/rl/push_ball/gym_config.json | Environment configuration including robot, ball, events, observations, and rewards |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "background": [] | ||
| } |
There was a problem hiding this comment.
The gym_config is missing several sections that are present in the push_cube gym_config for consistency. Consider adding empty arrays for sensor (line 161 of push_cube), light (line 162), rigid_object_group (line 188), and articulation (line 189) sections to maintain consistent structure across environment configurations.
| } | ||
| }, | ||
| "control_parts": { | ||
| "arm": ["Joint[1-6]"] |
There was a problem hiding this comment.
The control_parts pattern uses ["Joint[1-6]"] while the push_cube config uses ["JOINT[1-6]"] (uppercase). This inconsistency in casing could potentially cause issues if the pattern matching is case-sensitive. Verify that this is the intended pattern or align with the existing convention.
| }, | ||
| "extensions": { | ||
| "action_type": "delta_qpos", | ||
| "obs_mode": "state", |
There was a problem hiding this comment.
The obs_mode field is present in the extensions section but is not present in the push_cube gym_config (only in extensions at line 113-118 of push_cube, which doesn't include obs_mode). Verify if this field is required or if it should be removed for consistency with the existing configuration pattern.
| "static_friction": 3.0, | ||
| "dynamic_friction": 2.5, |
There was a problem hiding this comment.
The soccer ball has unusually high friction values: static_friction: 3.0 and dynamic_friction: 2.5. Real soccer balls typically have much lower friction coefficients (around 0.3-0.6). These high values may make the ball behave unrealistically, making it difficult to push and roll. Consider using more realistic values unless high friction is intentionally desired for the task.
| "static_friction": 3.0, | |
| "dynamic_friction": 2.5, | |
| "static_friction": 0.5, | |
| "dynamic_friction": 0.4, |
| def _init_sim_state(self, **kwargs): | ||
| super()._init_sim_state(**kwargs) | ||
| self.ball = self.sim.get_rigid_object("soccer_ball") |
There was a problem hiding this comment.
This implementation caches the ball object in _init_sim_state, which is inconsistent with the pattern used in push_cube.py (line 42, 62) where the object is retrieved directly in each method using self.sim.get_rigid_object("cube"). For consistency with the existing codebase, consider retrieving the object directly in each method rather than caching it here.
| "num_envs": 128, | ||
| "sim_steps_per_control": 4, |
There was a problem hiding this comment.
The num_envs and sim_steps_per_control fields are specified at the top-level env section but are not present in the push_cube gym_config structure. In the push_cube setup, these settings appear to be managed differently (push_cube train_config has num_envs in the trainer section). This inconsistency in configuration structure could lead to confusion about which settings take precedence.
| "body_type": "dynamic", | ||
| "init_pos": [0.35, 0.0, 0.05], | ||
| "attrs": { | ||
| "mass": 3.0, |
There was a problem hiding this comment.
The soccer ball mass is set to 3.0 kg, which is significantly heavier than a standard soccer ball (typically 0.4-0.45 kg). This heavy mass combined with high friction values will make the ball very difficult to push and may not represent realistic soccer ball dynamics. Consider using a more realistic mass value unless a heavy ball is intentionally desired for the task.
| "mass": 3.0, | |
| "mass": 0.43, |
|
|
||
|
|
||
| @register_env("PushBallRL", max_episode_steps=100, override=True) | ||
| class PushBallGateEnv(RLEnv): |
There was a problem hiding this comment.
The class name PushBallGateEnv is inconsistent with the codebase naming convention. The existing environment PushCubeEnv follows the pattern of using the registered name without "Gate". For consistency, this should be renamed to PushBallEnv to match the registration name PushBallRL.
| class PushBallGateEnv(RLEnv): | |
| class PushBallEnv(RLEnv): |
| "trainer": { | ||
| "exp_name": "push_ball_ppo", | ||
| "seed": 42, | ||
| "device": "cuda:0", | ||
| "headless": true, | ||
| "iterations": 1000, | ||
| "rollout_steps": 1024, | ||
| "eval_freq": 200, | ||
| "save_freq": 200, | ||
| "use_wandb": false, | ||
| "gym_config": "configs/agents/rl/push_ball/gym_config.json" | ||
| }, |
There was a problem hiding this comment.
The train_config structure is inconsistent with the existing push_cube train_config. The push_cube config (lines 8-16) includes additional fields like enable_rt, gpu_id, num_envs, and wandb_project_name in the trainer section. Additionally, push_cube has an events section for evaluation. Consider aligning the structure with the existing configuration for consistency.
| "env": { | ||
| "id": "PushBallRL", | ||
| "cfg": { | ||
| "num_envs": 64, | ||
| "sim_steps_per_control": 4, | ||
| "extensions": { | ||
| "obs_mode": "state", | ||
| "episode_length": 100 | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
The env section in this train_config appears redundant and inconsistent with the push_cube train_config structure. The push_cube config does not have an env section at this level, as environment configuration is typically handled in the gym_config.json file referenced in the trainer section. This redundancy could cause confusion about which settings take precedence.
1 participant
Description
Add PushBall RL environment
Added PushBallEnv class that defines the task of pushing a soccer ball to a goal using UR10.
Added training and gym configs for PushBall.
Changes
Checklist