[Task Clean-up] Dexterous Part 4/8: Fix MARL-to-single-agent training and enable handover Direct RSL-RL#6414
Conversation
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"
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ) | ||
|
|
||
| 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.
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.
_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.
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 |
| 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()) |
AntoineRichard
left a comment
AntoineRichard
left a comment
There was a problem hiding this comment.
My concern is that this PR is doing some real (important and needed) work on MARL and RSL-RL while hiding that under a RSL-RL only refactor.
Also same concerns around moving entirely to warp.
| self.goal_rot = torch.zeros((self.num_envs, 4), dtype=torch.float, device=self.device) | ||
| self.goal_rot[:, 0] = 1.0 | ||
| self.goal_rot[:, 3] = 1.0 # identity quaternion in (x, y, z, w) layout | ||
| self.goal_pos = torch.zeros((self.num_envs, 3), dtype=torch.float, device=self.device) |
There was a problem hiding this comment.
Why not use warp arrays directly?
Add a behavioral Metrics/success_rate signal (goal-reach streaks per episode) and threshold-independent episode orientation-error diagnostics to the Direct reorientation environments, with the shared helpers in isaaclab_tasks.core.utils and torch math tests. The task logic is torch-first per the mainline convention; success gates task health while reward stays diagnostic. Also fix hand resets that could initialize joints below their lower position limits.
Add the RSL-RL runner configuration for the Shadow handover Direct task and success-rate metrics on the torch-first path, fix handover construction on Newton (renamed distal joints), and land the camera Direct renderer presets with configuration validation. RSL-RL observations now read from the public environment-owned obs_buf on all Direct env bases (reset stores the buffer like step), replacing the adapter-side private hook.
5918610 to
392e7d9
Compare
hujc7
changed the title
hujc7
changed the title
…nager runtime (#6412) ## Summary - Fixes OVPhysX actuator joint indices to follow the common actuator indexing contract. - Fixes OVPhysX initialization alongside Kit by reusing Kit's registered PhysX schema provider. - Fixes the OVPhysX manager to support both the declared public runtime API and the current runtime API. - Regression tests included. Validated by full dexterous training runs on the OVPhysX backend; split out of the lumped validation branch #6324 (Part 2 of 11). ## Dependencies - None. ## Series review map Full integrated diff + training/validation evidence: the lumped validation PR #6324 (DO-NOT-MERGE). | Part | PR | |---|---| | Docs: regenerate the environment overview table | #6410 | | Part 1/11: Newton runtime fixes (cloner rows, cubric fallback, viz teardown) | #6411 | | **Part 2/11: OVPhysX runtime fixes (this PR)** | #6412 | | Part 3/11: success-rate metrics for the Direct reorientation tasks | #6413 | | Part 4/11: RSL-RL training for the handover Direct task | #6414 | | Part 5/11: success-rate support in the benchmark utilities | #6415 | | Part 6/11: renderer presets for the Direct camera task | #6416 | | Part 7/11: OVPhysX presets for the dexterous tasks | #6417 | | Part 8/11: Allegro manager counterpart | #6418 | | Part 9/11: Shadow + OpenAI manager counterparts | #6419 | | Part 10/11: Shadow camera manager counterpart | #6420 | | Part 11/11: Shadow handover manager counterpart | #6421 | --- ### Exact changes in this PR - OVPhysX backend changes + tests: 1f7a433
hujc7
changed the title
Fold the reviewed lump changes that belong to this part's content: - Share the handover sim settings through the HandoverTaskCfgBase mixin instead of a module-level cfg variable. - Reuse isaaclab.utils.math.quat_mul for the Newton initial-rotation composition instead of a local xyzw product helper. - Nest the single-consumer tiled-camera helper cfg inside the camera task cfg. - Drop the stale kit-less OpenUSD bullet from the isaaclab changelog fragment. Source commits on the lump branch: 2c22af0, 2c5ea5b, 42675b6, 1f05f85.
hujc7
changed the title
3 participants
Review Map
Summary
Scope note: the core of this PR is a fix to the multi-agent-to-single-agent adapter (
source/isaaclab/isaaclab/envs/utils/marl.py) and the RSL-RL vecenv wrapper — the adapter did not expose the latest observations through the public buffer, which silently breaks any MARL (Multi-Agent Reinforcement Learning) task trained through single-agent runners, not just handover. The handover Direct task is the exercised instance and gains RSL-RL training on top of the fix; the camera presets ride along for the camera Direct task.DirectMARLEnvtasks through themulti_agent_to_single_agentconversion, and that bridge was broken at the observation contract. This PR fixes it generally — RSL-RL observations read from the public environment-ownedobs_buf, stored byresetlikestepon all env bases (DirectRLEnv,DirectMARLEnvviaisaaclab.envs.utils.marl, and the experimental warp base), replacing the adapter-side private hook. Every MARL task paired with every single-agent runner benefits; the handover task is the first consumer, not the scope of the fix.Stacking
Validation