Using prismatic joints, with hard limits

[jeffeb3]

I'm trying to create my own model, and it requires modeling linear actuators. I'm assuming a prismatic joint is the right joint to use. I loaded it using an SDF file. But when I run the simulation, it just keeps falling through the limits I've set.

When I query the object's properties, I get the mPositionLowerLimit, mPositionUpperLimit are set to -1,0. These limits are on the PrismaticJointProperties.

What am I doing wrong?

[jslee02]

You might need to call Joint::setPositionLimitEnforced(true) to enable the constraints in dynamics simulation. This could be confusing in some sense, but we added this switch to be able to turn the position limit constraint on and off easily while the joint keeps the limit values.

It seems the parser should set the switch on when there are position limits by default, though.

[jeffeb3]

Oh thank you. That would have taken me a while to find. It would probably be good to have that note added to the documentation for the upper and lower limits.

Looking at the spec for SDF, it looks to me that it's "required" for a joint: http://sdformat.org/spec?ver=1.6&elem=joint

This particular SDF was created in gazebo's model creator, and it populated a limit for every joint, setting the default to -1e308,1e308. So if you setPositionLimitEnforced(true) when the <limit> is present, you should always get it set to true for SDF files. Is that OK? I mean, if someone doesn't adhere to the spec, and leaves out the limit, then dart's parser won't enforce a limit. If someone does follow the spec, and they don't actually want a limit, they will set it to 1e308. Which will be enforced, but not in this lifetime. Or they will set it to something they want, and it will be enforced.

[jeffeb3]

Oh, I was hoping I could help out, and add a line or two of code for review/PR, but it's not that simple, because it's not part of the properties, it's part of the joint. So the code that reads the limits is pretty far removed from the code that has a joint. I don't want to make that big a change without knowing the code better.

I would also add documentation to the limit properties, to let people know that they are ineffective without enforcing them.

[jslee02]

It would probably be good to have that note added to the documentation for the upper and lower limits.

:+1:

So if you setPositionLimitEnforced(true) when the is present, you should always get it set to true for SDF files. Is that OK?

We might don't want that case. One possible compromise I can come up with now is using threshhold. We make the joint to enforce position limits if the magnitute of the limits are less than the threshhold.

it's not part of the properties, it's part of the joint.

The function that reads the limits (e.g, readRevoluteJoint) takes members of RevoluteJoint::Properties. Since RevoluteJoint::Properties inherits Joint::Properties, we might be able to modify the function to take Joint::Properties::mIsPositionLimited as well. We welcome your contribution, but it is totally up you. So if it seems to introduce a big change then don't worry. :smiley:

[mkoval]

We might don't want that case. One possible compromise I can come up with now is using threshhold. We make the joint to enforce position limits if the magnitute of the limits are less than the threshhold.

What is the motivation behind having setPositionLimitEnforced at all? I would expect that finite joint limits would always be enforced. What is the difference between setPositionLimitEnforced(false) and setting the upper and lower limits to +/-INFINITY.

[jeffeb3]

@mkoval: jslee above pointed that there is a use case where you would want to disable the enforcement (presumably temporarily) and then add it back in later, without losing the values for the lower/upper. I can't vouch for the validity of that, but I'm sure there's a discussion here about it.

Using a threshold would make it easier. I see in the v1.4 version of the SDF spec that it has a default of -1e16,1e16, so that seems like a reasonable threshold: http://sdformat.org/spec?ver=1.4&elem=joint

I'm not sure what they mean when they say "required:1" because their example on the right doesn't even have it. And the description says you can omit it if the joint is continuous.

I can set it to -1e16,1e16 before the read, and if it's inside that threshold, then I'll enforce it. If that's a simple fix.

[mkoval]

@jeffeb3 Thanks! I see @jslee02's comment now:

This could be confusing in some sense, but we added this switch to be able to turn the position limit constraint on and off easily while the joint keeps the limit values.

Can you give an example where this is useful?

[jslee02]

Another motivation in the point of constraint solver view is related to the point @jeffeb3 made. The constraint solver doesn't generate constraints for a joint with setPositionLimitEnforced(false). So the difference is the constraint wouldn't care about a joint with setPositionLimitEnforced(false) and would generate constraint with a joint with +/-INFINITY. Then, every time step, the constraint solver checks if joint position exceeds the limits, and solve the constraint force when it exceeds.

I'm aware of that the way of generating (and managing) constraints in const solver is problematic since the constraint solver doesn't responses to the change of setPositionLimitEnforced. We should improve the logic of generating constraints of the constraint solver at some point. For example, there is no difference in generating constraints for static constraints and dynamic constraints (dynamic constraints can appear/disappear over simulation time).

[psigen]

Perhaps I am missing something:

I somewhat understand wanting to have a mechanism to temporarily disable joint limits without changing them.

[Note: Like @mkoval and @jeffeb3, I don't know when you would actually want this: every example I can come up with risks horrible consistency problems for trivial computational gains. Even if the solver is running a +/-INFINITY constraint every iteration, isn't this going to be wildly dominated by every other constraint that does more work than this if/else check?]

But OK, let's suppose there is some use case.

Why would we not want the default behavior for all loaders to be:

• Set joint limits if specified.
• Set +/-INFINITY for joints that don't have limits.
• Turn on setPositionLimitEnforced(true) for every joint.

Then people who have some specialized use-case for disabling joint limits can do so, and the average user-- who would not be doing this --does not have to dig around in the API to make their model work.

[mkoval]

I agree with everything @psigen said. Unless I'm missing a specific use case, I really think setPositionLimitEnforced should not exist at all.

The constraint solver doesn't generate constraints for a joint with setPositionLimitEnforced(false). So the difference is the constraint wouldn't care about a joint with setPositionLimitEnforced(false) and would generate constraint with a joint with +/-INFINITY.

This sounds like an efficiency problem in the constraint solver. I'd prefer to modify the ConstraintSolver to avoid creating constraints for +/-INFINITY joint limits. This is negligibly slower than checking isPositionLimitEnforced() each iteration.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[psigen]

@jslee02 : I just made a model with a prismatic joint, added joint limits to it, and spent 20 minutes trying to figure out why the prismatic joint limits were being completely ignored... until I found this open issue again. :sweat:

So I maintain that these should be on by default :grin:

[jslee02]

@psigen I agree that the current API is less intuitive and could cause confusion. Let me create a PR reflecting the comments here.