[Task Clean-up] Dexterous Part 3/8: Add success-rate metrics to the reorientation Direct tasks#6413
[Task Clean-up] Dexterous Part 3/8: Add success-rate metrics to the reorientation Direct tasks#6413hujc7 wants to merge 2 commits into
Conversation
Greptile SummaryThis PR adds behavioral
Confidence Score: 4/5Safe to merge; all new behaviour is additive (metrics, diagnostics, safer resets) and the reward math is functionally unchanged from the original. The core reward logic, success tracking, and joint-reset fix are all correct. The two findings are cosmetic: an unreachable torch.abs() on a provably non-negative value, and a repeated quaternion-distance computation (up to three times per step) that produces identical results each time. Neither affects training correctness or runtime safety. reorient_direct_env.py and mdp/rewards.py are worth a second look for the redundant evaluate_reorient_success calls; no other files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Env as ReorientDirectEnv
participant Dones as _get_dones()
participant Rewards as _get_rewards()
participant DRR as direct_reorient_reward()
participant Rec as EpisodeErrorRecorder
participant Reset as _reset_idx()
Env->>Dones: step()
Dones->>Dones: "evaluate_reorient_success() [if max_consecutive_success > 0]"
Dones-->>Env: (terminated, truncated)
Env->>Rewards: _get_rewards()
Rewards->>Rewards: evaluate_reorient_success() → orientation_error
Rewards->>Rec: update(orientation_error)
Rewards->>DRR: compute_rewards() → direct_reorient_reward()
DRR->>DRR: evaluate_reorient_success() [3rd call, same data]
DRR-->>Rewards: reward, goal_resets, successes, consecutive_successes
Rewards-->>Env: total_reward
Env->>Reset: _reset_idx(env_ids)
Reset->>Reset: "_last_episode_success = successes >= threshold"
Reset->>Rec: "reset(env_ids) → {mean, median, p90}"
Reset->>Reset: log Metrics/success_rate
Reset->>Reset: "log Diagnostics/episode_min_orientation_error_*"
Reset->>Reset: sample_joint_positions_within_limits()
Reset-->>Env: done
%%{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 Env as ReorientDirectEnv
participant Dones as _get_dones()
participant Rewards as _get_rewards()
participant DRR as direct_reorient_reward()
participant Rec as EpisodeErrorRecorder
participant Reset as _reset_idx()
Env->>Dones: step()
Dones->>Dones: "evaluate_reorient_success() [if max_consecutive_success > 0]"
Dones-->>Env: (terminated, truncated)
Env->>Rewards: _get_rewards()
Rewards->>Rewards: evaluate_reorient_success() → orientation_error
Rewards->>Rec: update(orientation_error)
Rewards->>DRR: compute_rewards() → direct_reorient_reward()
DRR->>DRR: evaluate_reorient_success() [3rd call, same data]
DRR-->>Rewards: reward, goal_resets, successes, consecutive_successes
Rewards-->>Env: total_reward
Env->>Reset: _reset_idx(env_ids)
Reset->>Reset: "_last_episode_success = successes >= threshold"
Reset->>Rec: "reset(env_ids) → {mean, median, p90}"
Reset->>Reset: log Metrics/success_rate
Reset->>Reset: "log Diagnostics/episode_min_orientation_error_*"
Reset->>Reset: sample_joint_positions_within_limits()
Reset-->>Env: done
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| orientation_error = direct_reorient_rotation_distance(object_quat, target_quat) | ||
| return torch.abs(orientation_error) <= success_tolerance, orientation_error |
There was a problem hiding this comment.
torch.abs() is redundant here. direct_reorient_rotation_distance computes 2 * arcsin(clamp(norm, max=1)), where the clamped norm is in [0, 1] and arcsin returns [0, π/2], so the product is always in [0, π]. Applying torch.abs on a provably non-negative tensor is a no-op and slightly misleads readers into thinking the value could be negative.
| orientation_error = direct_reorient_rotation_distance(object_quat, target_quat) | |
| return torch.abs(orientation_error) <= success_tolerance, orientation_error | |
| orientation_error = direct_reorient_rotation_distance(object_quat, target_quat) | |
| return orientation_error <= success_tolerance, orientation_error |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Do we want to move more of our logic to pure warp? Worth to discuss during our Thursday architecture meeting. I am also seeing dramatic speed-ups for newton when we start writing more kernels in the tasks. One of my concerns would mostly be on readability this is quite loaded, and some users may struggle to figure things out with this little information to go on. I think we should be a bit more didactic in the kernel descriptions.
| @wp.kernel | ||
| def _out_of_reach_kernel( | ||
| object_pos_w: wp.array(dtype=wp.vec3f), | ||
| env_origins: wp.array(dtype=wp.vec3f), | ||
| target_pos_e: wp.array(dtype=wp.vec3f), | ||
| fall_distance: float, | ||
| out_of_reach: wp.array(dtype=wp.bool), | ||
| ): | ||
| i = wp.tid() | ||
| out_of_reach[i] = wp.length(object_pos_w[i] - env_origins[i] - target_pos_e[i]) >= fall_distance | ||
|
|
||
|
|
||
| @wp.kernel | ||
| def _full_obs_kernel( | ||
| joint_pos: wp.array2d(dtype=wp.float32), | ||
| joint_vel: wp.array2d(dtype=wp.float32), | ||
| lower: wp.array2d(dtype=wp.float32), | ||
| upper: wp.array2d(dtype=wp.float32), | ||
| vel_scale: float, | ||
| object_pos_w: wp.array(dtype=wp.vec3f), | ||
| env_origins: wp.array(dtype=wp.vec3f), | ||
| object_quat: wp.array(dtype=wp.quatf), | ||
| object_lin_vel: wp.array(dtype=wp.vec3f), | ||
| object_ang_vel: wp.array(dtype=wp.vec3f), | ||
| in_hand_pos_e: wp.array(dtype=wp.vec3f), | ||
| goal_quat: wp.array(dtype=wp.quatf), | ||
| body_pos_w: wp.array2d(dtype=wp.vec3f), | ||
| body_quat_w: wp.array2d(dtype=wp.quatf), | ||
| body_vel_w: wp.array2d(dtype=wp.spatial_vectorf), | ||
| finger_ids: wp.array(dtype=wp.int32), | ||
| force: wp.array2d(dtype=wp.vec3f), | ||
| torque: wp.array2d(dtype=wp.vec3f), | ||
| wrench_ids: wp.array(dtype=wp.int32), | ||
| force_scale: float, | ||
| with_forces: int, | ||
| actions: wp.array2d(dtype=wp.float32), | ||
| out: wp.array2d(dtype=wp.float32), | ||
| ): | ||
| """Direct full observation / full state, matching the torch concatenation order. | ||
|
|
||
| Launched over ``(num_envs, obs_dim)``: each thread walks a branch ladder over the | ||
| segment boundaries and writes one output column, so warps (32 consecutive columns) | ||
| stay branch-uniform except at segment boundaries. | ||
| """ | ||
| i, j = wp.tid() | ||
| num_joints = joint_pos.shape[1] | ||
| num_fingers = finger_ids.shape[0] | ||
| # segment boundaries, in column order | ||
| end_joint = 2 * num_joints | ||
| end_object = end_joint + 13 | ||
| end_goal = end_object + 11 | ||
| end_tip_pos = end_goal + 3 * num_fingers | ||
| end_tip_quat = end_tip_pos + 4 * num_fingers | ||
| end_tip_vel = end_tip_quat + 6 * num_fingers | ||
| end_wrench = end_tip_vel | ||
| if with_forces != 0: | ||
| end_wrench += 6 * num_fingers | ||
| # hand: normalized DOF positions, scaled DOF velocities | ||
| if j < num_joints: | ||
| out[i, j] = 2.0 * (joint_pos[i, j] - lower[i, j]) / (upper[i, j] - lower[i, j]) - 1.0 | ||
| elif j < end_joint: | ||
| out[i, j] = vel_scale * joint_vel[i, j - num_joints] | ||
| # object pose and velocities (environment frame position) | ||
| elif j < end_object: | ||
| k = j - end_joint | ||
| if k < 3: | ||
| p = object_pos_w[i] - env_origins[i] | ||
| out[i, j] = p[k] | ||
| elif k < 7: | ||
| out[i, j] = object_quat[i][k - 3] | ||
| elif k < 10: | ||
| out[i, j] = object_lin_vel[i][k - 7] | ||
| else: | ||
| out[i, j] = vel_scale * object_ang_vel[i][k - 10] | ||
| # goal: in-hand anchor, goal rotation, and the goal-to-object rotation difference | ||
| elif j < end_goal: | ||
| k = j - end_object | ||
| if k < 3: | ||
| out[i, j] = in_hand_pos_e[i][k] | ||
| elif k < 7: | ||
| out[i, j] = goal_quat[i][k - 3] | ||
| else: | ||
| # quat_inverse == conjugate for these unit quaternions, matching | ||
| # isaaclab.utils.math.quat_mul/quat_conjugate semantics | ||
| qe = object_quat[i] * wp.quat_inverse(goal_quat[i]) | ||
| out[i, j] = qe[k - 7] | ||
| # fingertips: environment-frame positions, rotations, spatial velocities | ||
| elif j < end_tip_pos: | ||
| out[i, j] = fingertip_pos_col(body_pos_w, env_origins, finger_ids, i, j - end_goal) | ||
| elif j < end_tip_quat: | ||
| out[i, j] = fingertip_quat_col(body_quat_w, finger_ids, i, j - end_tip_pos) | ||
| elif j < end_tip_vel: | ||
| out[i, j] = fingertip_vel_col(body_vel_w, finger_ids, i, j - end_tip_quat) | ||
| # fingertip force/torque sensors (full state only; absent when with_forces == 0) | ||
| elif j < end_wrench: | ||
| if with_forces == 1: | ||
| k = j - end_tip_vel | ||
| c = k % 6 | ||
| if c < 3: | ||
| out[i, j] = force_scale * force[i, wrench_ids[k // 6]][c] | ||
| else: | ||
| out[i, j] = force_scale * torque[i, wrench_ids[k // 6]][c - 3] | ||
| else: | ||
| # full state requested but the sensor has no data yet: zero block | ||
| out[i, j] = 0.0 | ||
| # actions | ||
| else: | ||
| out[i, j] = actions[i, j - end_wrench] | ||
|
|
||
|
|
||
| @wp.kernel | ||
| def _reduced_obs_kernel( | ||
| body_pos_w: wp.array2d(dtype=wp.vec3f), | ||
| env_origins: wp.array(dtype=wp.vec3f), | ||
| finger_ids: wp.array(dtype=wp.int32), | ||
| object_pos_w: wp.array(dtype=wp.vec3f), | ||
| object_quat: wp.array(dtype=wp.quatf), | ||
| goal_quat: wp.array(dtype=wp.quatf), | ||
| actions: wp.array2d(dtype=wp.float32), | ||
| out: wp.array2d(dtype=wp.float32), | ||
| ): | ||
| """Direct reduced (OpenAI) observation, matching the torch concatenation order.""" | ||
| i = wp.tid() | ||
| num_fingers = finger_ids.shape[0] | ||
| for f in range(num_fingers): | ||
| fp = body_pos_w[i, finger_ids[f]] - env_origins[i] | ||
| out[i, 3 * f + 0] = fp[0] | ||
| out[i, 3 * f + 1] = fp[1] | ||
| out[i, 3 * f + 2] = fp[2] | ||
| idx = 3 * num_fingers | ||
| p = object_pos_w[i] - env_origins[i] | ||
| # quat_inverse == conjugate for these unit quaternions, matching | ||
| # isaaclab.utils.math.quat_mul/quat_conjugate semantics | ||
| qe = object_quat[i] * wp.quat_inverse(goal_quat[i]) | ||
| out[i, idx + 0] = p[0] | ||
| out[i, idx + 1] = p[1] | ||
| out[i, idx + 2] = p[2] | ||
| out[i, idx + 3] = qe[0] | ||
| out[i, idx + 4] = qe[1] | ||
| out[i, idx + 5] = qe[2] | ||
| out[i, idx + 6] = qe[3] | ||
| idx += 7 | ||
| for a in range(actions.shape[1]): | ||
| out[i, idx + a] = actions[i, a] |
There was a problem hiding this comment.
Why are they not in the re-orient kernel file?
There was a problem hiding this comment.
It's direct env specific. Is it still prefer to be moved given it's not used by others?
| self.successes = torch.zeros(self.num_envs, dtype=torch.float, device=self.device) | ||
| self.consecutive_successes = torch.zeros(1, dtype=torch.float, device=self.device) | ||
| self._last_episode_success = torch.zeros(self.num_envs, dtype=torch.bool, device=self.device) |
There was a problem hiding this comment.
If we are going full warp why not use wp.zeros?
| wp.launch( | ||
| ema_actuation_kernel, | ||
| dim=(self.num_envs, self._actuated_dof_ids_wp.shape[0]), | ||
| inputs=[ | ||
| wp.from_torch(self.actions), | ||
| self._lower_limits_wp, | ||
| self._upper_limits_wp, | ||
| self._actuated_dof_ids_wp, | ||
| self.cfg.act_moving_average, | ||
| self._prev_targets_wp, | ||
| self._cur_targets_wp, | ||
| ], | ||
| outputs=[self._compact_targets_wp], | ||
| device=self._compact_targets_wp.device, |
There was a problem hiding this comment.
For extra speeeeeed we could cache the kernel on the first exec using the record launch command.
There was a problem hiding this comment.
Will defer those for now. is there a util class merged?
| # RSL-RL holds the observation reference across the next env.step, so hand out | ||
| # a per-step snapshot rather than the persistent kernel output buffer. | ||
| observations = {"policy": obs.clone()} | ||
| if self.cfg.asymmetric_obs: | ||
| observations["critic"] = self.compute_full_state() | ||
| observations["critic"] = self.compute_full_state().clone() | ||
| return observations |
There was a problem hiding this comment.
Rather than cloning outside the kernel launch the kernel launch could write to safe pre-allocated warp arrays with cached torch bindings?
There was a problem hiding this comment.
rl lib could take the output and keep it more than one step. So properly using a persistent buffer would require a bit more changes. Deferring it for now.
| self.episode_length_buf = torch.where( | ||
| torch.abs(rot_dist) <= self.cfg.success_tolerance, | ||
| goal_reached, | ||
| torch.zeros_like(self.episode_length_buf), | ||
| self.episode_length_buf, | ||
| ) |
There was a problem hiding this comment.
Can't the kernel above do the torch.where?
| self.extras.setdefault("log", {})["Metrics/success_rate"] = ( | ||
| self._last_episode_success[env_ids].float().mean().item() | ||
| ) |
There was a problem hiding this comment.
This will force a cuda sync, there is a way to avoid this,
There was a problem hiding this comment.
What's the suggested way?
There was a problem hiding this comment.
Should have UTs on these kernels?
b1e2a42 to
cf21603
Compare
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.
ef2b22c to
6e8a63e
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
@AntoineRichard For this PR, I am updating the torch impl. The already implemented warp one is moving to experimental folder |
Fold the reviewed lump changes that belong to this part's content: - Share per-family sim settings through task-cfg base mixins. - Deduplicate backend scene presets via inner-class defaults and nest single-consumer helper cfgs in the shadow-hand Direct cfg. - Compute the orientation error through isaaclab.utils.math.quat_error_magnitude and delete the local direct_reorient_rotation_distance primitive. - Rename direct_reorient_reward to reorient_reward: shared symbols carry no paradigm prefix. Source commits on the lump branch: 2c22af0, 792e400, 42675b6, c6140f9, 173e9dc.
2 participants
Review Map
Summary
Metrics/success_ratesignal (goal-reach streaks per episode) and threshold-independent episode orientation-error diagnostics to the Direct reorientation environments; success gates task health, reward stays diagnostic..torchaccessors only at the core-lib boundary). This supersedes the earlier warp-first revision of this PR; the warp implementation moved toisaaclab_tasks_experimental([Task Clean-up] Dexterous Part 8/8: Move the warp Direct task variants to isaaclab_tasks_experimental #6582).isaaclab_tasks.core.utils(EpisodeErrorRecorder,sample_joint_positions_within_limits) with torch math tests; fixes hand resets below lower joint limits.Stacking
develop.Validation
Review history