Fixes actuator velocity limits propagation down the articulation root_physx_view

[jtigue-bdai]

Description

Previously the velocity limits of the the ActuatorBaseCfg do not get used in most actuators. Only the DCMotor actuator uses it but it only uses it for torque-speed limitations.

This PR propagates the velocity_limits of the ActuatorBaseCfg to the articulation root_physx_view.

Fixes #1384

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

[larsrpe]

This seems like a nice feature for consistency through the different layers. However, I wonder if this can have implications for the simulator performance, depending on how this constraint is implemented in the backend.

I think one potential issue is when using the DCMotor model we want to define a torque cutoff speed to obtain more realistic speed torque relationship, however this does not necessarily mean that we want to have a hard limit on the joint velocities, which might give simulation artefacts.

I think follow up work should be to separate the velocity limit used in the DC motor model and the one used for the hard physics limit. What do you think?

[jtigue-bdai]

That’s a good point. The velocity_limit is being used as the “no load speed” which works for active motor torques. But with external loading, motors can operate in another quadrant of the torque-speed curve producing more speed. It should be straightforward to not physically cap the velocity for the DCMotor.

…

On Tue, Dec 10, 2024 at 4:37 AM Lars Rønhaug Pettersen < ***@***.***> wrote: This seems like a nice feature for consistency through the different layers. However, I wonder if this can have implications for the simulator performance, depending on how this constraint is implemented in the backend. I think one potential issue is when using the DCMotor model we want to define a torque cutoff speed to obtain more realistic speed torque relationship, however this does not necessarily mean that we want to have a hard limit on the joint velocities, which might give simulation artefacts. I think follow up work should be to separate the velocity limit used in the DC motor model and the one used for the hard physics limit. What do you think? — Reply to this email directly, view it on GitHub <#1509 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BHV4FBM7IUFXONK5P4BEP7D2E2Y4LAVCNFSM6AAAAABTFJPSL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZRGAYTEMBXGQ> . You are receiving this because you were assigned.Message ID: ***@***.***>

[larsrpe]

Should we instead use self.noload_speed here:

IsaacLab/source/extensions/omni.isaac.lab/omni/isaac/lab/actuators/actuator_pd.py

Line 226 in 4ee4957

max_effort = self._saturation_effort * (1.0 - self._joint_vel / self.velocity_limit)

This would only require adding one field to the DCMotorCfg.

[Mayankm96]

I agree with @larsrpe. Solver-level limits are different from the explicit model considerations. For explicit models, we may want to do the clipping inside the model but not have PhysX apply any of that within it (as tight constraints may lead to undefined behaviors).

For most of the shared quantities, we should maybe have their counterpart quantities as "torque_limit_sim" and "velocity_limit_sim". This makes it clear that these are being set at the solver level always. Their default values can be high (as right now).