bowl and microwave env #24
Conversation
| "GoalReachingEnv", | ||
| "HangLifebuoyEnv", | ||
| "PutBowlInsideMicrowaveEnv", | ||
| "SweepTableEnv", |
examples/run_hang_lifebuoy_teleop.py
Outdated
| ) | ||
|
|
||
| # Start teleop wrapper (keyboard listener) FIRST, before creating Genesis scene | ||
| teleop_wrapper.start() # type: ignore |
| ) | ||
|
|
||
| # Start teleop wrapper (keyboard listener) FIRST, before creating Genesis scene | ||
| teleop_wrapper.start() # type: ignore |
examples/run_hang_lifebuoy_teleop.py
Outdated
| seed=0, | ||
| precision="32", | ||
| logging_level="info", | ||
| backend=gs.cpu, # type: ignore |
There was a problem hiding this comment.
i am not sure if we need type: ignore it here.
examples/run_sweep_table_teleop.py
Outdated
| seed=0, | ||
| precision="32", | ||
| logging_level="info", | ||
| backend=gs.cpu, # type: ignore |
examples/run_sweep_table_teleop.py
Outdated
| ) | ||
|
|
||
| # Start teleop wrapper (keyboard listener) FIRST, before creating Genesis scene | ||
| teleop_wrapper.start() # type: ignore |
| seed=0, | ||
| precision="32", | ||
| logging_level="info", | ||
| backend=gs.cpu, # type: ignore |
| except Exception as e: | ||
| # Skip entities that don't have get_pos/get_quat methods | ||
| print(f"Warning: Could not get pose for entity '{entity_name}': {e}") | ||
| continue |
There was a problem hiding this comment.
in stead of using try and except here, why don't we try to make sure the entity has get_pos and get_quat()
| device: torch.device = _DEFAULT_DEVICE, | ||
| ) -> None: | ||
| super().__init__(device=device) | ||
| self._device = device |
There was a problem hiding this comment.
this is not need. since in the BaseEnv, we already defined self.device: Final[torch.device] = device
| ), | ||
| show_viewer=True, # Enable viewer for visualization | ||
| show_FPS=False, | ||
| ) |
There was a problem hiding this comment.
the configuration (hyper)-parameters should be specified via EnvArgs
| pos=(0.0, 0.0, 0.05), | ||
| size=(0.6, 0.6, 0.1), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
same here, the numbers seems random?
| euler=(0, 0, 90), | ||
| scale=0.03, | ||
| collision=True, | ||
| ), |
| euler=(90, 0, 90), | ||
| scale=(10, 5, 10), | ||
| collision=True, | ||
| ), |
| def apply_action(self, action: torch.Tensor | Any) -> None: | ||
| """Apply action to the environment (BaseEnv requirement).""" | ||
| # For teleop, action might be a command object instead of tensor | ||
| if isinstance(action, torch.Tensor): |
There was a problem hiding this comment.
this if-else doesn't seem reasonable. How come if tensor is no action to applied? It is misleading and error-prone.
| ) -> None: | ||
| super().__init__(device=device) | ||
| self._device = device | ||
| self._num_envs = 1 # Single environment for teleop |
There was a problem hiding this comment.
the num_envs should not be hard-coded here.
|
|
||
|
|
||
| class PutBowlInsideMicrowaveEnv(BaseEnv): | ||
| """Put bowl inside microwave environment.""" |
There was a problem hiding this comment.
same also here for this environment. the environment configuration should be specified with EnvArgs. try to avoid using hard-coded parameters.
| def apply_action(self, action: torch.Tensor | Any) -> None: | ||
| """Apply action to the environment (BaseEnv requirement).""" | ||
| # For teleop, action might be a command object instead of tensor | ||
| if isinstance(action, torch.Tensor): |
|
|
||
|
|
||
| class SweepTableEnv(BaseEnv): | ||
| """Sweep table environment - sweep trashboxes to target zone.""" |
|
@LeonLiu4 not sure if the comments are addressed or there are any problems. I see there are new commits but I am not sure if I should review them. It would be nice to reply the comments, let us know if it is addressed or you have different opinions so we can move forward. |
I addressed your comments above. Since then I have added new features like changing the observation from tensor to dict and also added rgb images into the observations. |
Hi Leon, thanks a lot for addressing the comments. It generally better to not add new features on top of existing PRs since new features make it difficult for the reviewer to keep track new changes. As a recommendation, it is better to create smaller PRs. |
3 participants
No description provided.