Skip to content

Conversation

lorenwel
Copy link
Contributor

Description

The docstring of the IdealPDActuator didn't match its implementation. Desired and actual joint positions and velocities were swapped.

Actual implementation is like this:

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

It is "desired - current", the current docstring says the opposite:
image

Type of change

  • Documentation update

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • 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

@github-actions github-actions bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Sep 18, 2025
@Mayankm96 Mayankm96 added the documentation Improvements or additions to documentation label Sep 18, 2025
Copy link
Collaborator

@jtigue-bdai jtigue-bdai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good catch

@lorenwel
Copy link
Contributor Author

@Mayankm96 @jtigue-bdai Thanks for the review.
The code build step didn't run so I cannot merge this. How do I get it to trigger?

@Mayankm96 Mayankm96 merged commit 7455d3d into isaac-sim:main Sep 18, 2025
9 of 10 checks passed
kellyguo11 pushed a commit that referenced this pull request Sep 19, 2025
# Description

The docstring of the `IdealPDActuator` didn't match its implementation.
Desired and actual joint positions and velocities were swapped.

Actual implementation is like this:
```
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
```
It is "`desired - current`", the current docstring says the opposite: 
<img width="524" height="60" alt="image"
src="https://github.com/user-attachments/assets/efdc7348-1587-4ed6-be58-875e804e8db9"
/>


## Type of change

- Documentation update

## Checklist

- [x] I have read and understood the [contribution
guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
- [ ] 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
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

Co-authored-by: Lorenz Wellhausen <[email protected]>
ooctipus pushed a commit to ooctipus/IsaacLab that referenced this pull request Sep 20, 2025
# Description

The docstring of the `IdealPDActuator` didn't match its implementation.
Desired and actual joint positions and velocities were swapped.

Actual implementation is like this:
```
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
```
It is "`desired - current`", the current docstring says the opposite: 
<img width="524" height="60" alt="image"
src="https://github.com/user-attachments/assets/efdc7348-1587-4ed6-be58-875e804e8db9"
/>


## Type of change

- Documentation update

## Checklist

- [x] I have read and understood the [contribution
guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
- [ ] 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
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

Co-authored-by: Lorenz Wellhausen <[email protected]>
ooctipus pushed a commit to ooctipus/IsaacLab that referenced this pull request Sep 29, 2025
# Description

The docstring of the `IdealPDActuator` didn't match its implementation.
Desired and actual joint positions and velocities were swapped.

Actual implementation is like this:
```
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
```
It is "`desired - current`", the current docstring says the opposite: 
<img width="524" height="60" alt="image"
src="https://github.com/user-attachments/assets/efdc7348-1587-4ed6-be58-875e804e8db9"
/>


## Type of change

- Documentation update

## Checklist

- [x] I have read and understood the [contribution
guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
- [ ] 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
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

Co-authored-by: Lorenz Wellhausen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants