Fix zero agent for semantic action spaces - #7439
Conversation
Greptile SummaryThis PR introduces semantic neutral actions for manager-based controllers and updates checkpoint-free zero-agent playback to support composite and multi-agent action spaces. It also validates resolved task configurations before simulator startup.
Confidence Score: 5/5The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issue identified. The new neutral-action paths preserve the configured batched action structure, and the investigated controller commands round-trip through their current preprocessing for repository configurations. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Z[Zero-agent loop] --> K{Environment type}
K -->|Manager-based| M[ActionManager.neutral_actions]
M --> T[Concatenate term-specific neutral commands]
T --> E[Environment step]
K -->|Direct single-agent| S[Tensorize single action space with zero]
S --> E
K -->|Direct multi-agent| A[Tensorize each agent action space with zero]
A --> E
Reviews (1): Last reviewed commit: "Make zero agent use neutral actions" | Re-trigger Greptile |
There was a problem hiding this comment.
Isaac Lab Review Bot
The additive neutral_actions contract and zero-agent routing are coherent, but Pink IK constructs its neutral pose using the asset’s quaternion convention rather than the action term’s documented quaternion layout. This prevents the Pink neutral command from reliably holding the current orientation.
- Design and architecture: The base zero-valued default with controller-specific overrides provides a clean extension point, and
_get_neutral_actionsappropriately centralizes manager-based and direct-workflow behavior. The accepted concern is localized to Pink IK pose encoding rather than the overall design. - API:
ActionTerm.neutral_actionsandActionManager.neutral_actionsare additive properties with backward-compatible defaults. However, Pink IK documents action orientations as(x, y, z, w)while its override directly flattensbody_link_pose_w, whose quaternion uses(w, x, y, z), violating the new semantic contract. - Implementation: The direct and multi-agent fallbacks, pre-launch validation, and other controller overrides follow the intended paths. Pink IK must reorder the current body quaternion into the action term’s expected layout before flattening; the added test currently checks only quaternion magnitude and mirrors the implementation, so it does not detect this convention mismatch.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
| @property | ||
| def neutral_actions(self) -> torch.Tensor: | ||
| """Raw actions that hold the controlled frames and hand joints at their current state.""" | ||
| frame_poses = self._asset.data.body_link_pose_w.torch[:, self._controlled_frame_ids].clone() |
There was a problem hiding this comment.
🟡 Warning · Api — Pink neutral pose uses wrong quaternion order
body_link_pose_w stores orientation as (w, x, y, z), but this term documents its pose slots as position plus orientation (x, y, z, w) (see orientation_dim). Concatenating the raw body pose therefore hands process_actions a reordered quaternion, so the "hold current frames" command requests a different orientation than the current one. Convert the quaternion to the action term's documented layout before flattening.
This reverts commit 780dc7e.
|
run-ci |
Description
Fixes
zero_agent.pyfor tasks where literal zero actions are invalid or where the action space is not a flatBox.neutral_actionscontract to manager-based action terms.No new dependencies are required.
Type of change
Release backport
developScreenshots
Not applicable.
Validation
uv run pytest -q source/isaaclab/test/envs/test_neutral_actions.py source/isaaclab_rl/test/test_entrypoints.py— 20 passed in 2.33 seconds.uv run isaaclab -f— passed, including changelog validation.IsaacContrib-PickPlace-FixedBaseUpperBodyIK-G1-Abszero-agent reproduction for two steps without IK NaNs.Isaac-Reach-Franka-OSCfor two steps to validate absolute OSC neutral actions.IsaacContrib-Stack-Cube-UR10-Long-Suction-IK-Rel physics=newton_mjwarpexits before simulator launch with the PhysX-only surface-gripper guidance.Checklist
uv run isaaclab -fCONTRIBUTORS.md