search

rdiankov / openrave

Open Robotics Automation Virtual Environment: An environment for testing, developing, and deploying robotics motion planning algorithms.
http://www.openrave.org
Other
706 stars 344 forks source link

Draft: ignore NaN in joint values #1350

Closed YutaKojio closed 7 months ago

YutaKojio commented 8 months ago

This PR adds a NaN check for joint values so that we can set NaN to indicate another meaningful flag, not a value.

Also, this PR introduces a "max" interpolation method, which can be used for the group associated with NaN in joint_values group where an interpolation computation with NaN generates NaN. However, there might be a better approach because this is indirect.

rdiankov commented 8 months ago

So using NaN's in SetDOFValues is a way to preserve the old joint value? Wouldn't this make it harder to find bugs than solve problems..?

YutaKojio commented 8 months ago

So using NaN's in SetDOFValues is a way to preserve the old joint value? Wouldn't this make it harder to find bugs than solve problems..?

Yes. Do you mean Inf is preferable? or we should care NaN elsewhere? For Inf, the cons is that we'll need to add if( isinf(X) ) in more places so that arithmetic operations do not yield NaN and Inf is not clamped by max operation etc.

rdiankov commented 8 months ago

No, what I mean is that allowing to take NaN (or Inf) is going to make it harder to determine if the caller had a bug or that was the intent. What is the point of this MR?

YutaKojio commented 8 months ago

This MR is one of the ongoing internal MRs to add gripper values to openrave trajectory, which is the implementations that we discussed over email last October. With the implementation, a trajectory will have NaN value in a "joint_values" spec to represent async gripper movements.

However, now I agree that completely ignoring NaN in SetDOFValues is too much. So, possible improvements would be:

kanbouchou commented 7 months ago

@rdiankov could you check this comment https://github.com/rdiankov/openrave/pull/1350#issuecomment-1922605740 from Yuta? Based on our discussion before, I think we need a way of indicating invalid entry (NaN for example) into joint_values group of trajectory because we didn't want to add a dedicated configuration specification group for that.

rdiankov commented 7 months ago

after several conversations, having this functionality here will simplify a lot of other code, so let's put it in and see what happen. thanks~