RethinkRobotics-opensource / sns_ik

Saturation in the Null Space (SNS) Inverse Kinematic Library
82 stars 41 forks source link

remove #define for floating point numerical type #65

Closed MatthewPeterKelly closed 6 years ago

MatthewPeterKelly commented 6 years ago

Request: remove the pre-processor flag to switch between float and double in the core math utilities and standardize to using double-precision floating point throughout. @rethink-forrest @IanTheEngineer -- thoughts?

Details: In sns_ik_math_utils we use a pre-processor flag to switch between float and double for the core math utilities. This is a bad idea for several reasons:

The simplest solution would be to remove the pre-processor flag and use double precision floating point in the entire library.

Another solution would be to implement the entire library as a template library like Eigen. I do not suggest that we do this.

IanTheEngineer commented 6 years ago

This breaks the private API, rather than the public., so I don't see an issue with moving forward on your proposed change.

MatthewPeterKelly commented 6 years ago

Code changes in pull-request #70

MatthewPeterKelly commented 6 years ago

commit c1f5f7bbc9366995923ec8e92af575a931cbaa01 Author: Matthew Kelly matthew.kelly2@gmail.com Date: Mon May 7 15:33:29 2018 -0400

Remove preprocessor switch float vs double (#70)

* Remove preprocessor switch float vs double

The key change in this commit is to replace the preprocessor
flag that switched between using float and double precision.

It does not make any change to the implementation or documentation.

There were several small code clean-ups included as well:
- use standard Eigen typedefs (rather than custom)
- remove "using namespace Eigen"
- fix the scoping on local constants
- replace #define INF with numeric_limits<>::max()

*Code changes are based on the Google C++ style guide.