Skip to content

Conversation

matthewtrepte
Copy link
Contributor

@matthewtrepte matthewtrepte commented Sep 15, 2025

Description

Rename misnamed joint coefficient variables in Articulation as the new joint friction APIs are forces/torques and not coefficients.

In Articulation

  • Rename joint_friction_coeff to joint_static_friction_effort
  • Rename joint_dynamic_friction_coeff to joint_dynamic_friction_effort
  • Update a few comments / descriptions
  • Added proxies to preserve old name and mark them as deprecated

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Please attach before and after screenshots of the change if applicable.

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 the isaac-lab Related to Isaac Lab team label Sep 15, 2025
@kellyguo11 kellyguo11 changed the title rename joint dynamic and static friction variables Renames joint dynamic and static friction variables Sep 16, 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.

all looks good. the main thing is the use of torque. In most cases it will be torque but for prismatic joints it will be a force. Effort is a more general term but you can add clarification that effort (force or torque).

kellyguo11 and others added 11 commits September 17, 2025 10:28
Co-authored-by: James Tigue <[email protected]>
Signed-off-by: matthewtrepte <[email protected]>
Co-authored-by: James Tigue <[email protected]>
Signed-off-by: matthewtrepte <[email protected]>
Signed-off-by: matthewtrepte <[email protected]>
"""

# TODO(mtrepte): do these friction vars need to be renamed as well?
friction: dict[str, float] | float | None = None
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can mark this as deprecated as well and keep it for now for backward support for 4.5 where it's still a coefficient. we can update the docstring to mention in 4.5 it's coeff, and in 5.0+ it's effort. then, we can also create a new static_friction parameter for the new API that will be effort. @jtigue-bdai do you think that would make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, though please check again

@Mayankm96
Copy link
Contributor

Let's merge this after #3318 is done since that one has been hanging for a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
isaac-lab Related to Isaac Lab team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants