Unverified Commit a2540cd6 authored by Juana's avatar Juana Committed by GitHub

Uses `effort_limit` from USD if not specified in actuator cfg (#3522)

# Description

<!--
Thank you for your interest in sending a pull request. Please make sure
to check the contribution guidelines.

Link:
https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html

💡 Please try to keep PRs small and focused. Large PRs are harder to
review and merge.
-->

This PR fixes a bug in actuator initialization where effort limits
specified in USD assets were being incorrectly overridden with a very
large default value (1.0e9) for explicit actuator models.

Fixes # (issue)

Previously, the ActuatorBase initialization logic would unconditionally
fall back to _DEFAULT_MAX_EFFORT_SIM (1.0e9) for explicit actuator
models when effort_limit_sim was not explicitly set in the
configuration, even when the USD asset contained finite, meaningful
effort limit values.

## Type of change

<!-- As you go through the list, delete the ones that are not
applicable. -->

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


## Checklist

- [x] I have read and understood the [contribution
guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] 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

<!--
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
-->

---------
Signed-off-by: 's avatarJuana <yvetted@nvidia.com>
Signed-off-by: 's avatarKelly Guo <kellyg@nvidia.com>
Co-authored-by: 's avatarJames Tigue <166445701+jtigue-bdai@users.noreply.github.com>
Co-authored-by: 's avatargreptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: 's avatarKelly Guo <kellyg@nvidia.com>
parent 64681ea9
[package] [package]
# Note: Semantic Versioning is used: https://semver.org/ # Note: Semantic Versioning is used: https://semver.org/
version = "0.47.10" version = "0.47.11"
# Description # Description
title = "Isaac Lab framework for Robot Learning" title = "Isaac Lab framework for Robot Learning"
......
Changelog Changelog
--------- ---------
0.47.11 (2025-11-03)
~~~~~~~~~~~~~~~~~~~~
Fixed
^^^^^
* Fixed the bug where effort limits were being overridden in :class:`~isaaclab.actuators.ActuatorBase` when the ``effort_limit`` parameter is set to None.
* Corrected the unit tests for three effort limit scenarios with proper assertions
0.47.10 (2025-11-06) 0.47.10 (2025-11-06)
~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~
......
...@@ -55,13 +55,21 @@ class ActuatorBase(ABC): ...@@ -55,13 +55,21 @@ class ActuatorBase(ABC):
effort_limit: torch.Tensor effort_limit: torch.Tensor
"""The effort limit for the actuator group. Shape is (num_envs, num_joints). """The effort limit for the actuator group. Shape is (num_envs, num_joints).
For implicit actuators, the :attr:`effort_limit` and :attr:`effort_limit_sim` are the same. This limit is used differently depending on the actuator type:
- **Explicit actuators**: Used for internal torque clipping within the actuator model
(e.g., motor torque limits in DC motor models).
- **Implicit actuators**: Same as :attr:`effort_limit_sim` (aliased for consistency).
""" """
effort_limit_sim: torch.Tensor effort_limit_sim: torch.Tensor
"""The effort limit for the actuator group in the simulation. Shape is (num_envs, num_joints). """The effort limit for the actuator group in the simulation. Shape is (num_envs, num_joints).
For implicit actuators, the :attr:`effort_limit` and :attr:`effort_limit_sim` are the same. For implicit actuators, the :attr:`effort_limit` and :attr:`effort_limit_sim` are the same.
- **Explicit actuators**: Typically set to a large value (1.0e9) to avoid double-clipping,
since the actuator model already clips efforts using :attr:`effort_limit`.
- **Implicit actuators**: Same as :attr:`effort_limit` (both values are synchronized).
""" """
velocity_limit: torch.Tensor velocity_limit: torch.Tensor
...@@ -123,8 +131,11 @@ class ActuatorBase(ABC): ...@@ -123,8 +131,11 @@ class ActuatorBase(ABC):
are not specified in the configuration, then their values provided in the constructor are used. are not specified in the configuration, then their values provided in the constructor are used.
.. note:: .. note::
The values in the constructor are typically obtained through the USD schemas corresponding The values in the constructor are typically obtained through the USD values passed from the PhysX API calls
to the joints in the actuator model. corresponding to the joints in the actuator model; these values serve as default values if the parameters
are not specified in the cfg.
Args: Args:
cfg: The configuration of the actuator model. cfg: The configuration of the actuator model.
...@@ -196,7 +207,12 @@ class ActuatorBase(ABC): ...@@ -196,7 +207,12 @@ class ActuatorBase(ABC):
) )
self.velocity_limit = self._parse_joint_parameter(self.cfg.velocity_limit, self.velocity_limit_sim) self.velocity_limit = self._parse_joint_parameter(self.cfg.velocity_limit, self.velocity_limit_sim)
self.effort_limit = self._parse_joint_parameter(self.cfg.effort_limit, self.effort_limit_sim) # Parse effort_limit with special default handling:
# - If cfg.effort_limit is None, use the original USD value (effort_limit parameter from constructor)
# - Otherwise, use effort_limit_sim as the default
# Please refer to the documentation of the effort_limit and effort_limit_sim parameters for more details.
effort_default = effort_limit if self.cfg.effort_limit is None else self.effort_limit_sim
self.effort_limit = self._parse_joint_parameter(self.cfg.effort_limit, effort_default)
# create commands buffers for allocation # create commands buffers for allocation
self.computed_effort = torch.zeros(self._num_envs, self.num_joints, device=self._device) self.computed_effort = torch.zeros(self._num_envs, self.num_joints, device=self._device)
......
...@@ -1713,7 +1713,7 @@ class Articulation(AssetBase): ...@@ -1713,7 +1713,7 @@ class Articulation(AssetBase):
friction=self._data.default_joint_friction_coeff[:, joint_ids], friction=self._data.default_joint_friction_coeff[:, joint_ids],
dynamic_friction=self._data.default_joint_dynamic_friction_coeff[:, joint_ids], dynamic_friction=self._data.default_joint_dynamic_friction_coeff[:, joint_ids],
viscous_friction=self._data.default_joint_viscous_friction_coeff[:, joint_ids], viscous_friction=self._data.default_joint_viscous_friction_coeff[:, joint_ids],
effort_limit=self._data.joint_effort_limits[:, joint_ids], effort_limit=self._data.joint_effort_limits[:, joint_ids].clone(),
velocity_limit=self._data.joint_vel_limits[:, joint_ids], velocity_limit=self._data.joint_vel_limits[:, joint_ids],
) )
# log information on actuator groups # log information on actuator groups
......
...@@ -68,9 +68,7 @@ def test_ideal_pd_actuator_init_minimum(num_envs, num_joints, device, usd_defaul ...@@ -68,9 +68,7 @@ def test_ideal_pd_actuator_init_minimum(num_envs, num_joints, device, usd_defaul
torch.testing.assert_close(actuator.computed_effort, torch.zeros(num_envs, num_joints, device=device)) torch.testing.assert_close(actuator.computed_effort, torch.zeros(num_envs, num_joints, device=device))
torch.testing.assert_close(actuator.applied_effort, torch.zeros(num_envs, num_joints, device=device)) torch.testing.assert_close(actuator.applied_effort, torch.zeros(num_envs, num_joints, device=device))
torch.testing.assert_close( torch.testing.assert_close(actuator.effort_limit, torch.inf * torch.ones(num_envs, num_joints, device=device))
actuator.effort_limit, actuator._DEFAULT_MAX_EFFORT_SIM * torch.ones(num_envs, num_joints, device=device)
)
torch.testing.assert_close( torch.testing.assert_close(
actuator.effort_limit_sim, actuator._DEFAULT_MAX_EFFORT_SIM * torch.ones(num_envs, num_joints, device=device) actuator.effort_limit_sim, actuator._DEFAULT_MAX_EFFORT_SIM * torch.ones(num_envs, num_joints, device=device)
) )
...@@ -133,11 +131,11 @@ def test_ideal_pd_actuator_init_effort_limits(num_envs, num_joints, device, effo ...@@ -133,11 +131,11 @@ def test_ideal_pd_actuator_init_effort_limits(num_envs, num_joints, device, effo
effort_lim_sim_expected = actuator._DEFAULT_MAX_EFFORT_SIM effort_lim_sim_expected = actuator._DEFAULT_MAX_EFFORT_SIM
elif effort_lim is None and effort_lim_sim is not None: elif effort_lim is None and effort_lim_sim is not None:
effort_lim_expected = effort_lim_sim effort_lim_expected = effort_lim_default
effort_lim_sim_expected = effort_lim_sim effort_lim_sim_expected = effort_lim_sim
elif effort_lim is None and effort_lim_sim is None: elif effort_lim is None and effort_lim_sim is None:
effort_lim_expected = actuator._DEFAULT_MAX_EFFORT_SIM effort_lim_expected = effort_lim_default
effort_lim_sim_expected = actuator._DEFAULT_MAX_EFFORT_SIM effort_lim_sim_expected = actuator._DEFAULT_MAX_EFFORT_SIM
elif effort_lim is not None and effort_lim_sim is not None: elif effort_lim is not None and effort_lim_sim is not None:
......
...@@ -1372,10 +1372,16 @@ def test_setting_velocity_limit_explicit(sim, num_articulations, device, vel_lim ...@@ -1372,10 +1372,16 @@ def test_setting_velocity_limit_explicit(sim, num_articulations, device, vel_lim
@pytest.mark.parametrize("num_articulations", [1, 2]) @pytest.mark.parametrize("num_articulations", [1, 2])
@pytest.mark.parametrize("device", ["cuda:0", "cpu"]) @pytest.mark.parametrize("device", ["cuda:0", "cpu"])
@pytest.mark.parametrize("effort_limit_sim", [1e5, None]) @pytest.mark.parametrize("effort_limit_sim", [1e5, None])
@pytest.mark.parametrize("effort_limit", [1e2, None]) @pytest.mark.parametrize("effort_limit", [1e2, 80.0, None])
@pytest.mark.isaacsim_ci @pytest.mark.isaacsim_ci
def test_setting_effort_limit_implicit(sim, num_articulations, device, effort_limit_sim, effort_limit): def test_setting_effort_limit_implicit(sim, num_articulations, device, effort_limit_sim, effort_limit):
"""Test setting of the effort limit for implicit actuators.""" """Test setting of effort limit for implicit actuators.
This test verifies the effort limit resolution logic for actuator models implemented in :class:`ActuatorBase`:
- Case 1: If USD value == actuator config value: values match correctly
- Case 2: If USD value != actuator config value: actuator config value is used
- Case 3: If actuator config value is None: USD value is used as default
"""
articulation_cfg = generate_articulation_cfg( articulation_cfg = generate_articulation_cfg(
articulation_type="single_joint_implicit", articulation_type="single_joint_implicit",
effort_limit_sim=effort_limit_sim, effort_limit_sim=effort_limit_sim,
...@@ -1419,10 +1425,18 @@ def test_setting_effort_limit_implicit(sim, num_articulations, device, effort_li ...@@ -1419,10 +1425,18 @@ def test_setting_effort_limit_implicit(sim, num_articulations, device, effort_li
@pytest.mark.parametrize("num_articulations", [1, 2]) @pytest.mark.parametrize("num_articulations", [1, 2])
@pytest.mark.parametrize("device", ["cuda:0", "cpu"]) @pytest.mark.parametrize("device", ["cuda:0", "cpu"])
@pytest.mark.parametrize("effort_limit_sim", [1e5, None]) @pytest.mark.parametrize("effort_limit_sim", [1e5, None])
@pytest.mark.parametrize("effort_limit", [1e2, None]) @pytest.mark.parametrize("effort_limit", [80.0, 1e2, None])
@pytest.mark.isaacsim_ci @pytest.mark.isaacsim_ci
def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_limit_sim, effort_limit): def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_limit_sim, effort_limit):
"""Test setting of effort limit for explicit actuators.""" """Test setting of effort limit for explicit actuators.
This test verifies the effort limit resolution logic for actuator models implemented in :class:`ActuatorBase`:
- Case 1: If USD value == actuator config value: values match correctly
- Case 2: If USD value != actuator config value: actuator config value is used
- Case 3: If actuator config value is None: USD value is used as default
"""
articulation_cfg = generate_articulation_cfg( articulation_cfg = generate_articulation_cfg(
articulation_type="single_joint_explicit", articulation_type="single_joint_explicit",
effort_limit_sim=effort_limit_sim, effort_limit_sim=effort_limit_sim,
...@@ -1436,6 +1450,9 @@ def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_li ...@@ -1436,6 +1450,9 @@ def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_li
# Play sim # Play sim
sim.reset() sim.reset()
# usd default effort limit is set to 80
usd_default_effort_limit = 80.0
# collect limit init values # collect limit init values
physx_effort_limit = articulation.root_physx_view.get_dof_max_forces().to(device) physx_effort_limit = articulation.root_physx_view.get_dof_max_forces().to(device)
actuator_effort_limit = articulation.actuators["joint"].effort_limit actuator_effort_limit = articulation.actuators["joint"].effort_limit
...@@ -1452,8 +1469,9 @@ def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_li ...@@ -1452,8 +1469,9 @@ def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_li
# check physx effort limit does not match the one explicit actuator has # check physx effort limit does not match the one explicit actuator has
assert not (torch.allclose(actuator_effort_limit, physx_effort_limit)) assert not (torch.allclose(actuator_effort_limit, physx_effort_limit))
else: else:
# check actuator effort_limit is the same as the PhysX default # When effort_limit is None, actuator should use USD default values
torch.testing.assert_close(actuator_effort_limit, physx_effort_limit) expected_actuator_effort_limit = torch.full_like(physx_effort_limit, usd_default_effort_limit)
torch.testing.assert_close(actuator_effort_limit, expected_actuator_effort_limit)
# when using explicit actuators, the limits are set to high unless user overrides # when using explicit actuators, the limits are set to high unless user overrides
if effort_limit_sim is not None: if effort_limit_sim is not None:
...@@ -1462,6 +1480,7 @@ def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_li ...@@ -1462,6 +1480,7 @@ def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_li
limit = ActuatorBase._DEFAULT_MAX_EFFORT_SIM # type: ignore limit = ActuatorBase._DEFAULT_MAX_EFFORT_SIM # type: ignore
# check physx internal value matches the expected sim value # check physx internal value matches the expected sim value
expected_effort_limit = torch.full_like(physx_effort_limit, limit) expected_effort_limit = torch.full_like(physx_effort_limit, limit)
torch.testing.assert_close(actuator_effort_limit_sim, expected_effort_limit)
torch.testing.assert_close(physx_effort_limit, expected_effort_limit) torch.testing.assert_close(physx_effort_limit, expected_effort_limit)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment