[Bug Report] Issues with recently added action clipping for task space actions

[ozhanozen]

Hello IsaacLab Team,

While I was working on the OperationalSpaceController (#913), the new commit about clip ranges (#1476 ) caught my attention. Based on the commit message, this clipping mechanism appears to be intended for all actions, regardless of whether they are joint-based or task-based. This is not yet integrated into OperationalSpaceControllerAction. Moreover, I foresee challenges in extending it to task-space controller actions and a potential error with its current implementation for DifferentialInverseKinematicsAction.

For reference, here is how the clipping is currently implemented in DifferentialInverseKinematicsAction:

• construction
  

  IsaacLab/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/mdp/actions/task_space_actions.py

  Lines 111 to 120 in fc6042f

  	# parse clip
  	if self.cfg.clip is not None:
  	if isinstance(cfg.clip, dict):
  	self._clip = torch.tensor([[-float("inf"), float("inf")]], device=self.device).repeat(
  	self.num_envs, self.action_dim, 1
  	)
  	index_list, _, value_list = string_utils.resolve_matching_names_values(self.cfg.clip, self._joint_names)
  	self._clip[:, index_list] = torch.tensor(value_list, device=self.device)
  	else:
  	raise ValueError(f"Unsupported clip type: {type(cfg.clip)}. Supported types are dict.")

• application
  

  IsaacLab/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/mdp/actions/task_space_actions.py

  Lines 159 to 166 in fc6042f

  	if self.cfg.clip is not None:
  	self._processed_actions = torch.clamp(
  	self._processed_actions, min=self._clip[:, :, 0], max=self._clip[:, :, 1]
  	)
  	# obtain quantities from simulation
  	ee_pos_curr, ee_quat_curr = self._compute_frame_pose()
  	# set command into controller
  	self._ik_controller.set_command(self._processed_actions, ee_pos_curr, ee_quat_curr)

The issue lies in the way the clip is defined (i.e., using a dictionary) and constructed (i.e., modification based on joint indices), which strongly implies that the clip values correspond to joint limits, while the actions themselves are in task space. I suspect that L118 could already raise an error if self.action_dim does not equal the number of joints. Even if they are equal, applying joint limits to task-space actions is misleading and could lead to unintended behavior.

To address this, I propose the following hotfix:

1. Move the clip attribute of ActionTermCfg to JointActionCfg.
2. Update the documentation to explicitly state that clipping only applies to joint actions.
3. Remove the clipping logic from DifferentialInverseKinematicsAction.

Alternatively, if clipping for task-space actions is still required, additional considerations need to be addressed due to the variability in action definitions and specific implementations. These include:

1. Coordinate Naming: If a dictionary is used for partial clipping, what would the names of the task-space coordinates be? These can vary depending on the action type (e.g., absolute pose, relative pose, pos/pose/wrench).
2. Clipping Rotations: For task actions involving an absolute pose, how should the rotation component be clipped, given that it is defined as a quaternion?
3. Redundant Clipping: Some task-space action parameters (e.g., the impedance parameters of OperationalSpaceControllerAction) are not coordinate-related or are already subject to clipping. Should they be clipped again under the umbrella clip setting?

In my opinion, the responsibility for clipping task-space actions should remain with individual implementations, as the definitions and requirements for task-space actions vary significantly across different controllers and configurations.

Please let me know your thoughts. Thank you in advance.

System Info

• Commit: fc6042f
• Isaac Sim Version: 4.2
• OS: Ubuntu 22.04
• GPU: RTX 4090
• CUDA: 12.2
• GPU Driver: 535.129.03

Checklist

• I have checked that there is no similar issue in the repo (required)
• I have checked that the issue is not in running Isaac Sim itself and is related to the repo

Acceptance Criteria

• Either removing clipping from task space actions, or modifying it to fit task spaces.

[kellyguo11]

Thanks for raising the issue! It does look like there are some flaws in the dictionary logic for non-joint actions. It was actually originally implemented for JointAction, but we wanted to extend it towards all actions so we moved it to ActionTermCfg. I think there are still benefits to supporting clipping, but we'll need to reevaluate the dictionary representation. Seems like it might make sense to have more action-specific namings for the dictionary keys and the action class will then have to interpret it based on the action type.

[ozhanozen]

Yeah, that makes sense. Please let me know if there is anything I can support you with.

[kellyguo11]

Also related #1732