search

RobustFieldAutonomyLab / LeGO-LOAM

LeGO-LOAM: Lightweight and Ground-Optimized Lidar Odometry and Mapping on Variable Terrain
BSD 3-Clause "New" or "Revised" License
2.36k stars 1.11k forks source link

use Eigen instead of OpenCV (10% speed improvement) #105

Closed facontidavide closed 5 years ago

facontidavide commented 5 years ago

Hi,

I would like to submit exactly the same change that was submitted and accepted here:

https://github.com/laboshinl/loam_velodyne/pull/24

Since you seem to consider speed important, it is a pity to leave performance on the table ;)

Cheers

YoshuaNava commented 5 years ago

Hi, That's an interesting idea. Even though the PR is now closed, will you open it again?

Furthermore, does it change the performance in some other way than processing speed? For example accuracy.

facontidavide commented 5 years ago

change moved to PR #106

facontidavide commented 5 years ago

As far as I can tell, it has no effect on accuracy, but I would like the main maintainer to check that ;)

YoshuaNava commented 5 years ago

The main purpose of this package is localization. How do you submit a change without checking if it degrades performance?

Sorry if it sounds too harsh. Your contributions seem to be very nice.

facontidavide commented 5 years ago

@YoshuaNava

1) I think there is a misunderstanding. Of course I tried my code before submitting it.

2) Indeed, you have been a little bit harsh/rude, and I wwonder why I waste my time contributing to open source... anyway...

These are the results based on a one of the test rosbags:

Old one

image

Final odometry value

pose: 
    position: 
      x: 11.965464592
      y: 0.43265119195
      z: 58.6489067078
    orientation: 
      x: 0.0117278084569
      y: 0.860511307584
      z: 0.00680366999052
      w: 0.509250879335

New one (PR #106 )

image

Final odometry value

pose: 
    position: 
      x: 11.9772348404
      y: 0.390570670366
      z: 58.6788368225
    orientation: 
      x: 0.0114329593204
      y: 0.860528238123
      z: 0.00717487296129
      w: 0.509223880068

As you can see there is tiny difference. Which one is more correct? I can not know without the ground-truth.

It could be that my PR improved or worsen the performance a little bit. Or the standard deviation of error and the mean value are the same, considering multiple datasets.

Such a deep analysis is not possible for me because I don't have the ground-truth.

facontidavide commented 5 years ago

For the records. Running again the old code (on master) it seems that the final odometry value is not 100% deterministic.

Therefore it is not my change that introduce an "error" but the algorithm itself has some noises in the end-to-end odometry value.

YoshuaNava commented 5 years ago

The results looks very good.

About open source, I would say that code review is also a part of the process. I learn from you as a developer while I also try to share what I know, and help looking for pitfalls.