[Task Clean-up] Dexterous Part 4/11: Enable RSL-RL training for the handover Direct task#6414
[Task Clean-up] Dexterous Part 4/11: Enable RSL-RL training for the handover Direct task#6414hujc7 wants to merge 9 commits into
Conversation
The multi-agent-to-single-agent adapter now exposes current observations through the Direct RL interface used by RSL-RL (it previously returned stale buffers). Adds an RSL-RL PPO runner configuration and registers it on the Direct handover task, and adds the same success-rate metrics as the reorientation tasks. Also fixes the Newton distal-joint override to match the renamed distal joints (J0 -> J1) in the current Shadow Hand Newton asset, without which the handover environments fail to construct on Newton. Validated by full handover training on PhysX (success streaks met).
Greptile SummaryThis PR enables RSL-RL PPO training on the Shadow Hand handover Direct task by fixing the MARL-to-single-agent adapter to expose live observations via
Confidence Score: 3/5Not safe to merge as-is: The source/isaaclab_tasks/isaaclab_tasks/core/handover/handover_env.py — the import from Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant RSL as RSL-RL Runner
participant Adapter as multi_agent_to_single_agent (Env)
participant MARL as HandoverEnv (DirectMARLEnv)
RSL->>Adapter: reset(seed, options)
Adapter->>MARL: reset(seed, options)
MARL-->>Adapter: obs (per-agent dict), extras
Adapter->>Adapter: _convert_observations(obs)
Adapter-->>RSL: "{"policy": concat_obs}, extras"
RSL->>Adapter: _get_observations()
Note over Adapter: NEW: exposes live obs for RSL-RL direct access
Adapter->>MARL: _get_observations()
MARL-->>Adapter: obs (per-agent dict)
Adapter->>Adapter: _convert_observations(obs)
Adapter-->>RSL: "{"policy": concat_obs}"
RSL->>Adapter: step(action)
Adapter->>Adapter: split action by agent
Adapter->>MARL: "step({right_hand: a0, left_hand: a1})"
MARL-->>Adapter: obs, rewards, terminated, time_outs, extras
Adapter->>Adapter: _convert_observations(obs)
Adapter-->>RSL: obs, sum(rewards), AND(terminated), AND(time_outs), extras
RSL->>Adapter: "episode_length_buf = value"
Note over Adapter: Property setter forwards write to MARL env
Adapter->>MARL: "episode_length_buf = value"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant RSL as RSL-RL Runner
participant Adapter as multi_agent_to_single_agent (Env)
participant MARL as HandoverEnv (DirectMARLEnv)
RSL->>Adapter: reset(seed, options)
Adapter->>MARL: reset(seed, options)
MARL-->>Adapter: obs (per-agent dict), extras
Adapter->>Adapter: _convert_observations(obs)
Adapter-->>RSL: "{"policy": concat_obs}, extras"
RSL->>Adapter: _get_observations()
Note over Adapter: NEW: exposes live obs for RSL-RL direct access
Adapter->>MARL: _get_observations()
MARL-->>Adapter: obs (per-agent dict)
Adapter->>Adapter: _convert_observations(obs)
Adapter-->>RSL: "{"policy": concat_obs}"
RSL->>Adapter: step(action)
Adapter->>Adapter: split action by agent
Adapter->>MARL: "step({right_hand: a0, left_hand: a1})"
MARL-->>Adapter: obs, rewards, terminated, time_outs, extras
Adapter->>Adapter: _convert_observations(obs)
Adapter-->>RSL: obs, sum(rewards), AND(terminated), AND(time_outs), extras
RSL->>Adapter: "episode_length_buf = value"
Note over Adapter: Property setter forwards write to MARL env
Adapter->>MARL: "episode_length_buf = value"
|
| ) | ||
|
|
||
| from isaaclab_tasks.core.handover.handover_env_cfg import HandoverEnvCfg | ||
| from isaaclab_tasks.core.handover.mdp.rewards import evaluate_handover_success, handover_reward |
There was a problem hiding this comment.
Missing module:
isaaclab_tasks.core.handover.mdp.rewards
handover_env.py imports evaluate_handover_success and handover_reward from isaaclab_tasks.core.handover.mdp.rewards, but no such module exists anywhere in the repository — there is no mdp/ subdirectory under core/handover/. The PR description names PR #6413 as the only declared code dependency (for isaaclab_tasks.core.utils), but that PR's description only mentions "core.utils helpers" and does not describe adding a handover-specific mdp/rewards.py. Without this file the task class will raise ModuleNotFoundError on import, blocking all training runs.
| def _get_observations(self) -> VecEnvObs: | ||
| """Return current observations through the single-agent interface.""" | ||
| return self._convert_observations(self.env._get_observations()) |
There was a problem hiding this comment.
Unnecessary
_get_observations() call in state-as-observation mode
When _state_as_observation=True, _get_observations still calls self.env._get_observations() (which may trigger sensor data collection) and then immediately discards the result inside _convert_observations, which pivots to self.env.state(). The call is harmless but wasteful — consider short-circuiting before the inner call, e.g. returning {"policy": self.env.state()} directly when in state-as-observation mode.
The Direct handover environment imports handover_reward and evaluate_handover_success from the handover MDP package, so the multi-agent environment test could not construct the task without it. The manager-only remainder of the package lands with the manager counterpart PR.
Such implementation should not be accepted as it is trying to fix a problem a RL library has (require a private method, that is not part of the standard API Gymnasium / Petting-Zoo Parallel API to initialize its logic) from Isaac Lab side. There is nothing to fix here. The fix must be implemented in the RL library to do not depend on the private @kellyguo11 for viz |
Three stacked fixes validated by full training (success rate 0.74 by iteration 206 from a permanent flatline): per-backend actuated-joint names via the physics preset key, hand root rotations composed with the asset's baked base rotation instead of replacing it, and four solver substeps for sustained ball-palm contact. The shared handover reward functions move to Warp kernels with parity tests, and the multi-agent adapter skips computing observations it discards in state-as-observation mode.
Per-backend actuated-joint mapping for the renamed Newton asset, hand rotations composed with the asset's baked base rotation, raised contact substeps, Warp reward functions with parity tests, the skipped discarded-observation computation in the MARL adapter, and the shared handover constants module. Validated: success rate 0 -> ~0.95 on the previously flatlined handover-Newton row.
| def _convert_observations(self, obs: dict[AgentID, ObsType]) -> VecEnvObs: | ||
| """Convert multi-agent observations to the single-agent policy observation.""" | ||
| if self._state_as_observation: | ||
| obs = {"policy": self.env.state()} | ||
| # concatenate agents' observations | ||
| # FIXME: This implementation assumes the spaces are fundamental ones. Fix it to support composite spaces | ||
| else: | ||
| obs = { | ||
| "policy": torch.cat( | ||
| [obs[agent].reshape(self.num_envs, -1) for agent in self.env.possible_agents], dim=-1 | ||
| ) | ||
| } | ||
| return {"policy": self.env.state()} | ||
| return { | ||
| "policy": torch.cat( | ||
| [obs[agent].reshape(self.num_envs, -1) for agent in self.env.possible_agents], dim=-1 | ||
| ) | ||
| } | ||
|
|
||
| return obs, extras | ||
| def _get_observations(self) -> VecEnvObs: | ||
| """Return current observations through the single-agent interface.""" | ||
| if self._state_as_observation: | ||
| # the state replaces the observations entirely; skip computing them | ||
| return {"policy": self.env.state()} | ||
| return self._convert_observations(self.env._get_observations()) |
The scalar task parameters (decimation, episode length, reset noise, reward scales, thresholds) move into the constants module and the Direct cfg consumes them, so the manager counterpart can read the same values without instantiating the Direct cfg. The Warp reward wrappers take caller-owned buffers, the Newton hand rotation composes in float64 via wp.quatd, and the restored MARL FIXME marks the fundamental-space assumption at its consolidated site. Per review feedback, the MARL adapter no longer implements the private Direct-env observation hook: RslRlVecEnvWrapper caches the reset/step observations and serves get_observations() from the standard API.
The scalar task parameters (decimation, episode length, reset noise, reward scales, thresholds) move into the constants module and the Direct cfg consumes them, so the manager counterpart can read the same values without instantiating the Direct cfg. The Warp reward wrappers take caller-owned buffers, the Newton hand rotation composes in float64 via wp.quatd, and the restored MARL FIXME marks the fundamental-space assumption at its consolidated site. Per review feedback, the MARL adapter no longer implements the private Direct-env observation hook: RslRlVecEnvWrapper caches the reset/step observations and serves get_observations() from the standard API.
The scalar task parameters (decimation, episode length, reset noise, reward scales, thresholds) move into the constants module and the Direct cfg consumes them, so the manager counterpart can read the same values without instantiating the Direct cfg. The Warp reward wrappers take caller-owned buffers, the Newton hand rotation composes in float64 via wp.quatd, and the restored MARL FIXME marks the fundamental-space assumption at its consolidated site. Per review feedback, the MARL adapter no longer implements the private Direct-env observation hook: RslRlVecEnvWrapper reads the public observation buffer that every environment maintains, DirectRLEnv.reset stores that buffer like step already does, and the adapter exposes the wrapped environment's latest observations through the same attribute.
The scalar task parameters (decimation, episode length, reset noise, reward scales, thresholds) move into the constants module and the Direct cfg consumes them, so the manager counterpart can read the same values without instantiating the Direct cfg. The reward kernels launch directly on Warp-native environment state with caller-owned, view-cached buffers, the Newton hand rotation composes in float64 via wp.quatd, and the restored MARL FIXME marks the fundamental-space assumption at its consolidated site. Per review feedback, the MARL adapter no longer implements the private Direct-env observation hook: RslRlVecEnvWrapper reads the public observation buffer that every environment maintains, DirectRLEnv.reset stores that buffer like step already does, and the adapter exposes the wrapped environment's latest observations through the same attribute.
Summary
Isaac-Shadow-Handover-Direct, plus the same success-rate metrics as the reorientation tasks.Dependencies
isaaclab_tasks.core.utilshelpers imported by the handover env). The multi-agent environment suite cannot construct the handover task until [Task Clean-up] Dexterous Part 3/11: Add success-rate metrics to the reorientation Direct tasks #6413 merges, so CI on this PR stays red until then; the branch will be refreshed once it lands.Review updates (2026-07-09)
RslRlVecEnvWrapper.get_observations()now reads the public environment-owned observation buffer (obs_buf);DirectRLEnv.reset()stores the buffer likestep()already does (matching the manager-based and multi-agent environments), and the adapter exposes the wrapped environment's latest observations through the same public attribute. The private path also skipped observation-noise corruption, so initial observations now match what training receives.handover_task_constantsand the Direct cfg consumes them; Warp reward wrappers take caller-owned buffers; the Newton hand rotation composes in float64 viawp.quatd; stale J0 comment fixed.