Skip to content

Commit cdb1ddd

Browse files
committed
fix effort limit overwritten bug
1 parent 3466f2b commit cdb1ddd

File tree

3 files changed

+31
-16
lines changed

3 files changed

+31
-16
lines changed

source/isaaclab/isaaclab/actuators/actuator_base.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,7 @@ def __init__(
159159
self.joint_property_resolution_table: dict[str, list] = {}
160160
# For explicit models, we do not want to enforce the effort limit through the solver
161161
# (unless it is explicitly set)
162-
effort_limit_is_finite = (
163-
effort_limit != torch.inf if isinstance(effort_limit, float) else torch.all(effort_limit != torch.inf)
164-
)
165-
if not self.is_implicit_model and self.cfg.effort_limit_sim is None and (not effort_limit_is_finite):
162+
if not self.is_implicit_model and self.cfg.effort_limit_sim is None:
166163
self.cfg.effort_limit_sim = self._DEFAULT_MAX_EFFORT_SIM
167164

168165
# resolve usd, actuator configuration values
@@ -184,8 +181,6 @@ def __init__(
184181
cfg_val = getattr(self.cfg, param_name)
185182
setattr(self, param_name, self._parse_joint_parameter(cfg_val, usd_val))
186183
new_val = getattr(self, param_name)
187-
if param_name == "effort_limit_sim" or param_name == "effort_limit":
188-
print(f"effort_limit_sim: {new_val}, usd_val: {usd_val}, cfg_val: {cfg_val}")
189184

190185
allclose = (
191186
torch.all(new_val == usd_val) if isinstance(usd_val, (float, int)) else torch.allclose(new_val, usd_val)
@@ -201,9 +196,9 @@ def __init__(
201196
)
202197

203198
self.velocity_limit = self._parse_joint_parameter(self.cfg.velocity_limit, self.velocity_limit_sim)
204-
self.effort_limit = self._parse_joint_parameter(self.cfg.effort_limit, self.effort_limit_sim)
205-
206-
print(f"after: {self.effort_limit}, {self.effort_limit_sim}, cfg_val: {self.cfg.effort_limit}")
199+
# For effort_limit, use the tensor passed to constructor if cfg.effort_limit is None
200+
effort_default = self.effort_limit_sim if self.cfg.effort_limit is not None else effort_limit
201+
self.effort_limit = self._parse_joint_parameter(self.cfg.effort_limit, effort_default)
207202

208203
# create commands buffers for allocation
209204
self.computed_effort = torch.zeros(self._num_envs, self.num_joints, device=self._device)

source/isaaclab/isaaclab/assets/articulation/articulation.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1681,7 +1681,7 @@ def _process_actuators_cfg(self):
16811681
friction=self._data.default_joint_friction_coeff[:, joint_ids],
16821682
dynamic_friction=self._data.default_joint_dynamic_friction_coeff[:, joint_ids],
16831683
viscous_friction=self._data.default_joint_viscous_friction_coeff[:, joint_ids],
1684-
effort_limit=self._data.joint_effort_limits[:, joint_ids],
1684+
effort_limit=self._data.joint_effort_limits[:, joint_ids].clone(),
16851685
velocity_limit=self._data.joint_vel_limits[:, joint_ids],
16861686
)
16871687
# log information on actuator groups
@@ -1811,6 +1811,7 @@ def _apply_actuator_model(self):
18111811
"""
18121812
# process actions per group
18131813
for actuator in self.actuators.values():
1814+
18141815
# prepare input for actuator model based on cached data
18151816
# TODO : A tensor dict would be nice to do the indexing of all tensors together
18161817
control_action = ArticulationActions(

source/isaaclab/test/assets/test_articulation.py

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,10 +1372,16 @@ def test_setting_velocity_limit_explicit(sim, num_articulations, device, vel_lim
13721372
@pytest.mark.parametrize("num_articulations", [1, 2])
13731373
@pytest.mark.parametrize("device", ["cuda:0", "cpu"])
13741374
@pytest.mark.parametrize("effort_limit_sim", [1e5, None])
1375-
@pytest.mark.parametrize("effort_limit", [1e2, None])
1375+
@pytest.mark.parametrize("effort_limit", [1e2, 80.0, None])
13761376
@pytest.mark.isaacsim_ci
13771377
def test_setting_effort_limit_implicit(sim, num_articulations, device, effort_limit_sim, effort_limit):
1378-
"""Test setting of the effort limit for implicit actuators."""
1378+
"""Test setting of effort limit for implicit actuators.
1379+
1380+
This test verifies the effort limit resolution logic for actuator models implemented in :class:`ActuatorBase`:
1381+
- Case 1: If USD value == actuator config value: values match correctly
1382+
- Case 2: If USD value != actuator config value: actuator config value is used
1383+
- Case 3: If actuator config value is None: USD value is used as default
1384+
"""
13791385
articulation_cfg = generate_articulation_cfg(
13801386
articulation_type="single_joint_implicit",
13811387
effort_limit_sim=effort_limit_sim,
@@ -1419,10 +1425,18 @@ def test_setting_effort_limit_implicit(sim, num_articulations, device, effort_li
14191425
@pytest.mark.parametrize("num_articulations", [1, 2])
14201426
@pytest.mark.parametrize("device", ["cuda:0", "cpu"])
14211427
@pytest.mark.parametrize("effort_limit_sim", [1e5, None])
1422-
@pytest.mark.parametrize("effort_limit", [1e2, None])
1428+
@pytest.mark.parametrize("effort_limit", [80.0, 1e2, None])
14231429
@pytest.mark.isaacsim_ci
14241430
def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_limit_sim, effort_limit):
1425-
"""Test setting of effort limit for explicit actuators."""
1431+
"""Test setting of effort limit for explicit actuators.
1432+
1433+
This test verifies the effort limit resolution logic for actuator models implemented in :class:`ActuatorBase`:
1434+
- Case 1: If USD value == actuator config value: values match correctly
1435+
- Case 2: If USD value != actuator config value: actuator config value is used
1436+
- Case 3: If actuator config value is None: USD value is used as default
1437+
1438+
"""
1439+
14261440
articulation_cfg = generate_articulation_cfg(
14271441
articulation_type="single_joint_explicit",
14281442
effort_limit_sim=effort_limit_sim,
@@ -1436,6 +1450,9 @@ def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_li
14361450
# Play sim
14371451
sim.reset()
14381452

1453+
# usd default is 80
1454+
usd_default = 80.0
1455+
14391456
# collect limit init values
14401457
physx_effort_limit = articulation.root_physx_view.get_dof_max_forces().to(device)
14411458
actuator_effort_limit = articulation.actuators["joint"].effort_limit
@@ -1452,8 +1469,9 @@ def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_li
14521469
# check physx effort limit does not match the one explicit actuator has
14531470
assert not (torch.allclose(actuator_effort_limit, physx_effort_limit))
14541471
else:
1455-
# check actuator effort_limit is the same as the PhysX default
1456-
torch.testing.assert_close(actuator_effort_limit, physx_effort_limit)
1472+
# When effort_limit is None, actuator should use USD default values
1473+
expected_actuator_effort_limit = torch.full_like(physx_effort_limit, usd_default)
1474+
torch.testing.assert_close(actuator_effort_limit, expected_actuator_effort_limit)
14571475

14581476
# when using explicit actuators, the limits are set to high unless user overrides
14591477
if effort_limit_sim is not None:
@@ -1462,6 +1480,7 @@ def test_setting_effort_limit_explicit(sim, num_articulations, device, effort_li
14621480
limit = ActuatorBase._DEFAULT_MAX_EFFORT_SIM # type: ignore
14631481
# check physx internal value matches the expected sim value
14641482
expected_effort_limit = torch.full_like(physx_effort_limit, limit)
1483+
torch.testing.assert_close(actuator_effort_limit_sim, expected_effort_limit)
14651484
torch.testing.assert_close(physx_effort_limit, expected_effort_limit)
14661485

14671486

0 commit comments

Comments
 (0)