RSDK-9076 make ik functions more generic #4467
Conversation
…k-on-framesystems-rather-than-individual-frames
viambot
added
the
safe to test
Availability
Quality
Performance
The above data was generated by running scenes defined in the
|
| @@ -59,21 +55,24 @@ type optimizeReturn struct { | |||
| // CreateNloptIKSolver creates an nloptIK object that can perform gradient descent on metrics for Frames. The parameters are the Frame on | |||
There was a problem hiding this comment.
[nit] comment should be updated here
| if len(gradient) > 0 { | ||
| // Yes, the for loop below is logically equivalent to not having this if statement. But CPU branch prediction means having the | ||
| // if statement is faster. | ||
| for i := range gradient { | ||
| jumpVal = jump[i] | ||
| flip := false | ||
| inputs[i].Value += jumpVal | ||
| x[i] += jumpVal |
There was a problem hiding this comment.
[nit] should use something more descriptive than x
| PTG | ||
|
|
||
| // Returns the set of trajectory nodes along the given trajectory, out to the requested distance. | ||
| // This will return `TrajNode`s starting at dist=start, and every `resolution` increments thereafter, and finally at `end` exactly. | ||
| // This could |
There was a problem hiding this comment.
Whoops. Not sure where that came from!
| logger logging.Logger | ||
| solvers []InverseKinematics | ||
| logger logging.Logger | ||
| lowerBound []float64 |
There was a problem hiding this comment.
since this is "in preparation for motion planning supporting solving for framesystem states in general" wouldn't it make more sense for lowerBound to be of type [][]float64?
Is the overall motivation of supporting solving for framesystem states so that we can plan for coordinated motion?
What is general motivation of this work?
There was a problem hiding this comment.
We don't want [][]float64, because ik still operates on arrays of floats; it is simply a generic function rather than specifically a Frame on which Transform is called.
The overall motivation is to make it easy for motionplan to solve for framesystem states, yes. But that is not the same thing as actually passing framesystems into ik, that's not what we want. We just want ik here to be a generic function, so that we may create a distance function which measures a frame system, a single frame, or something else unrelated to frames.
| for i, l := range lowerBound { | ||
| u := upperBound[i] | ||
|
|
||
| // Default to [-999,999] as range if limits are infinite |
There was a problem hiding this comment.
why this range and not [-9999,9999]?
why these numbers specifically?
[opt] 999 could probably be turned into a constant?
There was a problem hiding this comment.
This is essentially a random number. There's no reason for 999 or 9999 except that 999 is what we've always used. This function has simply moved from being a method on nlopt.
We need something to set as the limits in the event of infinity, and the actual value chosen should not matter very much as gradient descent should take care of it.
Agree this should be a constant.
This PR removes the concept of Frames from IK, in preparation for motion planning supporting solving for framesystem states in general rather than simply the pose of one component relative to another.
This involves making some minor tweaks to our PTG and InverseKinematics interfaces, and also alters nlopt IK removing the restricted solving range, something that was very arm-centric and questionably useful