k-4pcs registration method as an extension of 4pcs

[theilerp]

as proposed, I split up the pull requests into two parts. this part includes the extension of 4pcs called k-4pcs as proposed by Theiler (2014). note, that this pull request is only valid if #976 has previously been merged...

[taketwo]

What is the state of converged_ in this case?

[taketwo]

Would be nice to have this covered with unit tests.

[theilerp]

Generally, I can commit a unit test. But the problem is as follows; k-4pcs combines the idea of "intelligent" point cloud reduction (by extracting keypoints) with the 4pcs matching strategy. Thus the k-4pcs is not desigend to work with the available data (point clouds) in the test section of pcl, but rather to enable fast matching for larger point clouds which first need a reduction.

Of course we shouldn't add laser scans of 500 mb size to the unit tests, thus I can see this solution: I provide two point clouds which include the e.g. SIFT keypoints with reasonable parameter setting of some test data I am working with; the unit test is then based on these point clouds.

Would that be an option?

[VictorLamoine]

More work for you:

ia_kfpcs.h:135: warning: argument 'vector' of command @param is not found in the argument list of pcl::registration::KFPCSInitialAlignment< PointSource, PointTarget, NormalT, Scalar >::getNBestCandidates(int n, float min_angle3d, float min_translation3d, MatchingCandidates &candidates)
ia_kfpcs.h:135: warning: The following parameters of pcl::registration::KFPCSInitialAlignment::getNBestCandidates(int n, float min_angle3d, float min_translation3d, MatchingCandidates &candidates) are not documented:
  parameter 'candidates'
ia_kfpcs.h:146: warning: argument 'vector' of command @param is not found in the argument list of pcl::registration::KFPCSInitialAlignment< PointSource, PointTarget, NormalT, Scalar >::getTBestCandidates(float t, float min_angle3d, float min_translation3d, MatchingCandidates &candidates)
ia_kfpcs.h:146: warning: The following parameters of pcl::registration::KFPCSInitialAlignment::getTBestCandidates(float t, float min_angle3d, float min_translation3d, MatchingCandidates &candidates) are not documented:
  parameter 'candidates'
ia_kfpcs.h:195: warning: argument 'ori_mat' of command @param is not found in the argument list of pcl::registration::KFPCSInitialAlignment< PointSource, PointTarget, NormalT, Scalar >::validateTransformation(Eigen::Matrix4f &transformation, float &fitness_score)
ia_kfpcs.h:195: warning: argument 'score' of command @param is not found in the argument list of pcl::registration::KFPCSInitialAlignment< PointSource, PointTarget, NormalT, Scalar >::validateTransformation(Eigen::Matrix4f &transformation, float &fitness_score)
ia_kfpcs.h:195: warning: The following parameters of pcl::registration::KFPCSInitialAlignment::validateTransformation(Eigen::Matrix4f &transformation, float &fitness_score) are not documented:
  parameter 'transformation'
  parameter 'fitness_score'

[taketwo]

Would that be an option?

Sounds good. Actually there are other valid reasons to include precomputed keypoints into test data. Firstly, unit test for this class should be isolated from keypoint extraction class, i.e. it should not break if keypoint extraction breaks. Secondly, keypoint extraction may be expensive, and we are already hitting Travis time limit more often than not.

[theilerp]

Are there any further issues to deal with? Or is it ready to merge?

[taketwo]

pcl_kdtree listed two times (and in the previous test as well)

[taketwo]

Apart from my inline comment I think we are good to go.

[theilerp]

Anything else or ist is ready to merge? I'm sorry if I am pressing here, but I have some requests about sharing the code and I just mentioned that it will soon be integrated into the PCL trunk ^^

[jspricke]

There are still two pcl_kdtree in registration/include/pcl/registration/CMakeLists.txt (two times). Also, @taketwo mentions inline comments and once it's done, please squash your commits.

[theilerp]

Maybe I did a pull/push messup, but I removed the two pcl_kdtree definitions, or am I missing something?

Regarding the inline comment, I added an additional node line to the method description, should be enough, right?

[jspricke]

As you can see here: theilerp@86fcee5, you removed them from test/registration/CMakeLists.txt, but no registration/include/pcl/registration/CMakeLists.txt. I guess the comments are ok.

[theilerp]

Ah, sorry for that. I definitely was confused ^^

and squashed

[theilerp]

Travis tests are failing all the time, but I guess not because of the submitted code, right?

Is there anything else to correct before merging?

[VictorLamoine]

Ignore Travis result unless there is a clear compiler/linker error in the log. In your case, the build was fine until Travis crashed; nothing do to with your code.

[taketwo]

It seems you have included an extra file "registration/include/pcl/registration/CMakeLists.txt".

[theilerp]

strange, but now I understand the problem I had of working out the previous comment about kdtree, because I did remove it in this unintended copy instead of in the original one ^^.

Should be fine now, I will squash everything, as soon as you give the okay...

[taketwo]

LGTM. Sorry for the delays in reviewing.

[theilerp]

No worries, I know you also have other things to do ^^

Ready to merge?