Fixes gear assembly environment for Isaac Sim 6.0 and XYZW quaternions

[ashwinvkNV]

Description

Fix gear assembly environment for Isaac Sim 6.0 and XYZW quaternions

[greptile-apps]

Greptile Overview

Greptile Summary

This PR updates several task environments and utilities to be compatible with Isaac Sim 6.0’s XYZW quaternion convention by replacing isaacsim.core.utils.torch helpers (tf_combine, tf_inverse, quat_*, Euler conversions, unscale) with isaaclab.utils.math equivalents (combine_frame_transforms, quat_inv, quat_apply, quat_mul, quat_from_euler_xyz, euler_xyz_from_quat, unscale_transform). It also updates Jacobian access in the gear assembly deploy events to use root_physx_view.

Key areas touched:

• Direct automate tasks (assembly/disassembly) and FORGE now use combine_frame_transforms (pos-first return) and XYZW quaternion ops.
• Tactile sensor SDF query path now computes inverse transforms with quat_inv/quat_apply and uses transform_points.
• Gear assembly deploy config changes grasp quaternion ordering to XYZW.

Fixes needed before merge:

• disassembly_env.py sets robot_to_gripper_quat to [1,0,0,0], which is not identity in XYZW and will rotate the target pose.
• mdp/events.py:set_robot_to_grasp_pose writes joint state using a joint_vel that can be stale or not defined after the IK loop.
• Config/docs mismatch: grasp rotation offset is now XYZW in the config, but validation messaging still claims WXYZ, which will mislead users.

Confidence Score: 2/5

• This PR should not be merged until a couple of task-breaking logic issues are corrected.
• Most changes are mechanical migrations to isaaclab.utils.math for XYZW quaternions, but at least two concrete issues remain: disassembly_env uses a non-identity quaternion for robot_to_gripper_quat and the gear-assembly deploy IK event can write joint state with stale/undefined joint_vel after the loop. There is also a config/documentation mismatch on quaternion ordering that can cause incorrect setups.
• source/isaaclab_tasks/isaaclab_tasks/direct/automate/disassembly_env.py; source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/deploy/mdp/events.py; source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/deploy/gear_assembly/config/ur_10e/joint_pos_env_cfg.py

Important Files Changed

Filename	Overview
source/isaaclab_contrib/isaaclab_contrib/sensors/tacsl_sensor/visuotactile_sensor.py	Replaced isaacsim.core.utils.torch tf_inverse/tf_apply with isaaclab.utils.math (quat_inv/quat_apply/transform_points) for XYZW quaternions; logic appears consistent.
source/isaaclab_tasks/isaaclab_tasks/direct/automate/assembly_env.py	Migrated transform/quaternion ops from torch_utils to isaaclab.utils.math and adjusted combine_frame_transforms return ordering for XYZW; no clear functional regressions found.
source/isaaclab_tasks/isaaclab_tasks/direct/automate/disassembly_env.py	Similar math migration as assembly_env, but robot_to_gripper_quat is set to [1,0,0,0] (not identity in XYZW), likely rotating goals by 180°.
source/isaaclab_tasks/isaaclab_tasks/direct/automate/factory_control.py	Switched quaternion math to isaaclab.utils.math and fixed scalar index to w component in XYZW; change is consistent with updated convention.
source/isaaclab_tasks/isaaclab_tasks/direct/forge/forge_env.py	Replaced torch_utils quaternion/euler helpers with isaaclab.utils.math equivalents for XYZW; updates appear coherent.
source/isaaclab_tasks/isaaclab_tasks/direct/forge/forge_utils.py	Reimplemented FT frame conversion using quat_inv/quat_apply/combine_frame_transforms; transform math matches XYZW utilities.
source/isaaclab_tasks/isaaclab_tasks/direct/locomotion/locomotion_env.py	Replaced torch_utils.maths.unscale with isaaclab.utils.math.unscale_transform and switched quat_conjugate import; change is straightforward.
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/deploy/gear_assembly/config/ur_10e/joint_pos_env_cfg.py	Updated grasp_rot_offset ordering comment/value to XYZW, but conflicts with downstream validation/docs still claiming WXYZ.
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/deploy/mdp/events.py	Switched jacobian access to root_physx_view.get_jacobians; post-IK write uses potentially stale/undefined joint_vel and has grasp_rot_offset doc mismatch.

Sequence Diagram

sequenceDiagram
    participant Env as Direct/Manager Env
    participant Math as isaaclab.utils.math
    participant Ctrl as factory_control.get_pose_error
    participant Sim as Sim/PhysX Views

    Env->>Sim: Read poses/quats (root/body)
    Env->>Math: combine_frame_transforms(t01,q01,t12,q12)
    Math-->>Env: (t02,q02) in XYZW
    Env->>Ctrl: get_pose_error(curr_pos,curr_quat,target_pos,target_quat)
    Ctrl->>Math: quat_conjugate/quat_mul/axis_angle_from_quat
    Ctrl-->>Env: pos_error, axis_angle_error
    Env->>Sim: Get jacobians (root_view/root_physx_view)
    Env->>Ctrl: _get_delta_dof_pos(delta_pose, jacobian)
    Ctrl-->>Env: delta joint positions
    Env->>Sim: write_joint_state_to_sim / set targets
    Env->>Sim: step + update buffers

Loading

[greptile-apps]

_{9 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (3)

source/isaaclab_tasks/isaaclab_tasks/direct/forge/forge_utils.py
Incorrect frame combine order

combine_frame_transforms takes (pos_a, quat_a, pos_b, quat_b) and returns (pos_ab, quat_ab). Here target_frame is documented as a (quat, pos) tuple, but the call passes target_frame[1] as position and target_frame[0] as quaternion, which is correct only if the tuple is actually (pos, quat). Given the function docstring says (quat, pos), this will compute the wrong transform (and thus wrong torques/forces) at runtime. Please either fix the tuple order in the docstring and all callers, or swap the arguments here to match the documented (quat, pos) convention.

---

source/isaaclab_tasks/isaaclab_tasks/direct/forge/forge_env.py
Zeroing quaternion components

This code sets self.noisy_fingertip_quat[:, [0, 3]] = 0.0 after switching to XYZW math utilities. In XYZW format, indices 0 and 3 are x and w; zeroing w produces a non-unit quaternion (often invalid/undefined orientation) and can make downstream euler_xyz_from_quat / axis-angle computations unstable. If the intent was to zero roll/pitch noise or enforce planar rotation, it should be done by constraining Euler angles or by zeroing the appropriate vector components before converting to quaternion, not by clobbering quaternion components.

---

source/isaaclab_tasks/isaaclab_tasks/direct/automate/assembly_env.py
Keypoint transform swap

combine_frame_transforms returns (pos, quat), but this loop uses [0] for the keypoint position while passing keypoint_offset as the position and identity_quat as the quat. If keypoint_offset is meant to be a point in the held-base frame, it should be transformed with a point transform (e.g., transform_points) rather than composing two frames. As written, it computes a composed frame origin, which differs from transforming the keypoint vector when the base has rotation (nonzero quat). This will yield incorrect keypoints_held/fixed distances when objects rotate.

[kellyguo11]

Is this using the Articulation class? I thought we renamed root_physx_view to root_view - https://github.com/isaac-sim/IsaacLab/blob/develop/source/isaaclab_physx/isaaclab_physx/assets/articulation/articulation.py#L170

[AntoineRichard]

Yes, there is a deprecation cycle on the physx side, so this property is still available object on the physx specialized assets. TBH, I feel like this task should likely import the PhysX version of this directly.

[AntoineRichard]

Thanks for migrating the environment after the quaternion change! My main concern, is all the commented code in the envs cfg. Do we need to keep it? Or should we set it to None in the Rizon arm? Typically we use the None pattern in the locomotion envs, but maybe it's not needed anymore for the other arms?

[greptile-apps]

_{9 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (2)

source/isaaclab_tasks/isaaclab_tasks/direct/automate/disassembly_env.py
Wrong identity quaternion

robot_to_gripper_quat is set to [1, 0, 0, 0] at disassembly_env.py:129-132, which is not an identity quaternion in the XYZW convention used throughout these updated math utilities (identity is [0, 0, 0, 1]). This will rotate the gripper goal by 180° and will change the task behavior vs the assembly env (and vs the previous torch_utils path).

---

source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/deploy/mdp/events.py
Stale joint velocity written

In events.py:349-366, joint_vel is last set inside the IK loop, but after the loop you re-clone joint_pos (line ~349) and then call write_joint_state_to_sim(joint_pos, joint_vel, ...) (line ~360) without re-initializing joint_vel for the current post-loop joint_pos. If the loop breaks early, joint_vel may be undefined for this scope or inconsistent with the final joint_pos, causing incorrect sim state writes or a runtime error.