Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug Report] The torque calculation formula in the documentation is inconsistent with the actual code implementation #1643

Closed
Limorettle opened this issue Jan 8, 2025 · 3 comments · Fixed by #1668
Labels
documentation Improvements or additions to documentation

Comments

@Limorettle
Copy link

Describe the bug

"The torque calculation formula in the documentation is inconsistent with the actual code implementation, which may cause confusion or errors."

Error details

In the documentation, the formula for calculating torque is:

$$\tau_{j, computed} = k_p * (q - q_{des}) + k_d * (\dot{q} - \dot{q}_{des}) + \tau_{ff}$$

Whereas in the code implementation:

def compute(
        self, control_action: ArticulationActions, joint_pos: torch.Tensor, joint_vel: torch.Tensor
    ) -> ArticulationActions:
        # compute errors
        error_pos = control_action.joint_positions - joint_pos
        error_vel = control_action.joint_velocities - joint_vel
        # calculate the desired joint torques
        self.computed_effort = self.stiffness * error_pos + self.damping * error_vel + control_action.joint_efforts

There are the following inconsistencies between the formula and the code:

The directions of the position error and velocity error are opposite.
In the documentation:
$(q - q_{des}),(\dot{q} - \dot{q}_{des})$

In the code:

$(q_{des} - q),(\dot{q}_{des} - \dot{q})$

This inconsistency may lead to misunderstandings or incorrect controller behavior. I would like to clarify the following points:

Is there a unified definition of the error direction in both the formula and the code?
If the code implementation is correct, should the formula in the documentation be revised to $(q_{des} - q)$ and $(\dot{q}_{des} - \dot{q})$?
If the formula in the documentation is correct, should the code be modified to match the formula?
Looking forward to your response. Thank you!

System Info

Describe the characteristic of your environment:

  • Commit: [8f3b9ca]
  • Isaac Sim Version: [4.2.0]
  • OS: [Ubuntu 20.04]
  • GPU: [RTX 4070 Ti Super]
  • CUDA: [12.2]
  • GPU Driver: [535.171.04]
@RandomOakForest
Copy link
Collaborator

Thanks for posting this. If you could share the link to the doc you are referring to would be appreciated. Thanks.

@RandomOakForest RandomOakForest added the documentation Improvements or additions to documentation label Jan 8, 2025
@Limorettle
Copy link
Author

Thanks for posting this. If you could share the link to the doc you are referring to would be appreciated. Thanks.

In this class omni.isaac.lab.actuators.IdealPDActuator. The compute torque formula in the doc is inconsistent with the actual code implementation.

In the doc:

$$ \tau_{j, computed} = k_p * (q - q_{des}) + k_d * (\dot{q} - \dot{q}{des}) + \tau{ff} $$

In the code:

def compute(
        self, control_action: ArticulationActions, joint_pos: torch.Tensor, joint_vel: torch.Tensor
    ) -> ArticulationActions:
        # compute errors
        error_pos = control_action.joint_positions - joint_pos
        error_vel = control_action.joint_velocities - joint_vel
        # calculate the desired joint torques
        self.computed_effort = self.stiffness * error_pos + self.damping * error_vel + control_action.joint_efforts
        # clip the torques based on the motor limits
        self.applied_effort = self._clip_effort(self.computed_effort)
        # set the computed actions back into the control action
        control_action.joint_efforts = self.applied_effort
        control_action.joint_positions = None
        control_action.joint_velocities = None
        return control_action

Parameters:

    control_action – The joint action instance comprising of the desired joint positions, joint velocities and (feed-forward) joint efforts.

    joint_pos – The current joint positions of the joints in the group. Shape is (num_envs, num_joints).

    joint_vel – The current joint velocities of the joints in the group. Shape is (num_envs, num_joints).

@kellyguo11
Copy link
Contributor

Thanks for spotting this. The code implementation should be correct, we'll update the docs.

AntoineRichard pushed a commit to AntoineRichard/IsaacLab that referenced this issue Jan 15, 2025
# Description

There was a mismatch in the implementation of the IdealPDActuator code
and the equation provided in the actuators docs. This change fixes the
ordering of parameters in the docs to match the implementation in code.

Fixes isaac-sim#1643 

## Type of change

- This change requires a documentation update

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [ ] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

<!--
As you go through the checklist above, you can mark something as done by
putting an x character in it

For example,
- [x] I have done this task
- [ ] I have not done this task
-->
hapatel-bdai pushed a commit that referenced this issue Jan 21, 2025
# Description

There was a mismatch in the implementation of the IdealPDActuator code
and the equation provided in the actuators docs. This change fixes the
ordering of parameters in the docs to match the implementation in code.

Fixes #1643 

## Type of change

- This change requires a documentation update

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [ ] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

<!--
As you go through the checklist above, you can mark something as done by
putting an x character in it

For example,
- [x] I have done this task
- [ ] I have not done this task
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants