Looks like there are 2 PointToPlaneWithCovErrorMinimizer.

[artivis]

I might be wrong but it looks like PR #220 is not effective.

In the Registry class, the former PointToPlaneWithCovErrorMinimizer is still the one registered :

Line 109 :
ADD_TO_REGISTRAR(ErrorMinimizer, PointToPlaneWithCovErrorMinimizer, PointToPlaneWithCovErrorMinimizer<T>)

should be

ADD_TO_REGISTRAR(ErrorMinimizer, PointToPlaneWithCovErrorMinimizer, typename ErrorMinimizersImpl<T>::PointToPlaneWithCovErrorMinimizer).

Edit:

Well changing the Registry as proposed above leads to compilation error :

error: invalid new-expression of abstract class type 'ErrorMinimizersImpl<float>::PointToPlaneWithCovErrorMinimizer'
...
note:   because the following virtual functions are pure within 
...
note: 	PointMatcher<T>::TransformationParameters PointMatcher<T>::ErrorMinimizer::compute(const PointMatcher<T>::ErrorMinimizer::ErrorElements&)

Did I miss something ?

[pomerlef]

For the next two months, I won't have access to my development tools. It would be great if you could dig a bit more into this.

[pomerlef]

@davidlandry93 Is that related to your changes?

[davidlandry93]

From #208 PointToPlaneWithCov error minimizer used to live in it's own file in the pointmatcher/ErrorMinimizers directory. The rationale was to keep the files smaller to build and have a better separation of concerns.

After #220 the PointToPlaneWithCov error minimizer was copied back into the ErrorMinimizersImpl file but was not removed from pointmatcher/ErrorMinimizers/PointToPlaneWithCov.cpp.

I don't know why #220 was made. I thought I preserved the interface when I moved the ErrorMinimizer in a separate directory. Maybe I broke it and the author of #220 wanted to fix it? Or maybe we just got messed up in our branches. Perhaps @tomifischer is still around to help.

[tomifischer]

Hi, PR #220 was introduced as a fix to issue #36 since optimization covariance was not working on 2D scenarios. I guess branches got mixed up. It's probably safe to move it back to it's own file.

[goldbattle]

It looks like this removed the covariance estimation for the 2d lidar pointcloud that was added in PR #220. It looks like the force2D param isn't used any more in the current code since that was deleted in pr #276. Was there a plan to re-add this in? @simonpierredeschenes @tomifischer

[protobits]

hi, I have the same question as @goldbattle
It seems #276 either broke or altered functionality introduced in #220 allowing for covariance estimation in 2D case. Any chance that this could be revisited (or that PR be reverted if indeed it broke #220)? Or how is the functionality introduced in #220 accessible now?