search

Jmeyer1292 / opw_kinematics

Closed form IK for parallel base, spherical wrist industrial manipulators
Apache License 2.0
69 stars 26 forks source link

Affine -> Isometry #10

Closed Jmeyer1292 closed 5 years ago

Jmeyer1292 commented 6 years ago

I fairly recently learned that the ubiquitous use of Affine3d in code related to MoveIt is a bit of a mistake. We'd prefer to use Isometry which covers only rigid body transforms.

This makes the change for this library which is young enough to make switches. I welcome thoughts though.

The use of Eigen at all is really not needed for this library. Maybe we should make a transform matrix wrapper? One motivation would be using this library in code on a GPU (though CUDA for instance). I don't know if Eigen works there.

VictorLamoine commented 6 years ago

Be careful because using an Isometry does NOT guarantee that the matrix will stay an Isometry: https://stackoverflow.com/questions/46176533/how-to-ensure-eigen-isometry-stays-isometric

The change is useful (eg: there is no reason to implement shear operations on a robot pose) but it won't change much (maybe a little performance improvement is to be expected).

gavanderhoorn commented 6 years ago

+1 to the proposed change, the Affine in MoveIt is strange (but apparently historical).

And +1 to the comments by @VictorLamoine.

re: gpu and introducing a wrapper: Eigen already explicitly supports vectorisation, so unless we're expecting millions of parallel queries, GPUs might be nice-to-have but not very necessary?