Improve RadiusOutlierRemoval's performance #709

Falcon4 · 2014-05-28T09:15:32Z

Change search & check method.
radius-serach & check k --> k-search & check radius

Falcon4 · 2014-05-28T09:17:53Z

Performance became at least twice faster than present

VictorLamoine · 2014-05-28T09:43:36Z

Hey, thanks for your contribution ! I've seen your post on the PCL user list;
Why did you close the pull request ? If you can; open it again, otherwise make a new one 😉

Falcon4 · 2014-05-28T10:53:51Z

Sory.
I don't understand how to use a pull request well.
Now I reopen it.

VictorLamoine · 2014-05-28T20:32:51Z

There is something wrong with your code. The results are not the same and its much slower!
https://gist.github.com/VictorLamoine/f7470a7f1faf2559d7b0#file-radiusoutlierremoval

Original code

Point cloud size: 307200
Computation time: 1265 ms
Cloud filtered size: 212758

Your code (it takes an eternity!)

Point cloud size: 307200
Computation time: 559581 ms
Cloud filtered size: 280883

Falcon4 · 2014-05-29T04:43:57Z

Dear Victor Lamoine.

I test RadiusOutlierRemoval performance again, but I can’t replicate below situation.
https://gist.github.com/Falcon4/9c23f6dd9754a081589a

I send my data file.
https://github.com/Falcon4/CloudDataForTest
If you send your data to me, I will try it.

Best regard.

VictorLamoine · 2014-05-29T16:06:48Z

Give it a try, compile and install your PCL version with the modifications then test with your PCD file;

Original code:

$ ./radiusOutlierRemoval CloudRaw01.pcd 
Point cloud size: 307200
Computation time: 3153 ms
Cloud filtered size: 242000

Your code:

[100%] Built target radiusOutlierRemoval
Point cloud size: 307200
Computation time: 586252 ms
Cloud filtered size: 307186

It's not normal that I don't get the same results, and it took nearly 10 minutes!

Falcon4 · 2014-06-02T08:13:46Z

Hi

I tried your test code, but runtime error is occured in organized.hpp line 57.
So, I remove a NaN value from a point cloud data before filter processing, and get a different result.
https://github.com/Falcon4/CloudDataForTest

[Original code]
Point cloud size: 242014
Computation time: 60892 ms
Cloud filterd size: 242000

[My code]
Point cloud size: 242014
Computation time: 11041 ms
Cloud filterd size: 242000

I changed a computer ,OS, and IDE.
Computer: Dell Precision T7500 (Intel Xeon Quad) --> Dell Precision T3400 (Intel Core2 Dual)
OS :Window 7 64bit --> Windows XP 32bit
IDE :Qt creator --> Visual Studio 2010

I think that NaN test in pcl::search::OrganizedNeighbor::radiusSearch() is ignored in Linux.
If you remove a NaN value before filtering process, you will get a non eternity result.
Please tyr it.

If NaN value removal code is needed like a statistical_outlier_removal.hpp line 97,
Please tell me how to treat a NaN value.

Cheers

VictorLamoine · 2014-06-04T07:53:13Z

Hello,

Ok I can reproduce your results now:

Original code

$ ./radius_outlier_removal ../CloudRaw01.pcd
Point cloud size: 307200
Computation time: 3196 ms
Cloud filtered size: 242000

$ ./radius_outlier_removal ../CloudRaw01.pcd
Point cloud size: 307200
Computation time: 3204 ms
Cloud filtered size: 242000

$ ./radius_outlier_removal ../CloudRaw01_noNaN.pcd
Point cloud size: 242014
Computation time: 5086 ms
Cloud filtered size: 242000

$ ./radius_outlier_removal ../CloudRaw01_noNaN.pcd
Point cloud size: 242014
Computation time: 5068 ms
Cloud filtered size: 242000

Falcon4 code:

$ ./radius_outlier_removal ../CloudRaw01.pcd
Point cloud size: 307200
Computation time: 175009 ms
Cloud filtered size: 307186

$ ./radius_outlier_removal ../CloudRaw01_noNaN.pcd
Point cloud size: 242014
Computation time: 1054 ms
Cloud filtered size: 242000

When using a point cloud with NaN the results are not the same between the original/Falcon4 code
Your code is slower when using a point cloud with NaN (only on Linux ?)
Your code is faster when using a point cloud with no NaN values.

There are still things that need to be fixed!
Bye

Falcon4 · 2014-06-05T06:28:14Z

Hi

I add a NaN skip code to applyFilterIndices function, and reproduce your result too.
A point cloud with NaN is slower than a point cloud without NaN.
https://github.com/Falcon4/CloudDataForTest

[Point cloud without NaN]
Point cloud size: 242014
Computation time:11203 ms
Cloud filterd size:24200

[Point cloud with NaN]
Point cloud size: 307200
Computation time: 87659 ms
Cloud filterrs size:24200

I coud't test Original, Because runtime error occured.
See RuntimeErrorOriginal.JPG and error_in_organized_hpp.JPG

I tested StaticalOutlierRemoval, and get similar result too.
I think that Flann's k-search can't treat NaN data well

Bye

taketwo · 2014-06-07T12:35:42Z

So what is the conclusion after all? Will we actually downgrade performance for at least some users?

Falcon4 · 2014-06-09T01:09:43Z

Hi

If PCL request that "user must remove NaN value before filtering", my code is faster than original.
And StaticalOutlierRemoval will be faster too.
I think that organized.hpp line 57 request it.

I don't know which is better.(To be or not to be.)
I think that it is a PCL's policy.

Bye

Falcon4 · 2014-06-09T04:16:50Z

Hi

Is it possible to remove NaN data in setInputCloud() function? (override)
I think that performance is better than original and get same result in Linux.
And Inprove StaticalOutlierRemoval performance too.

If it 's OK, I will try it.

Bye

VictorLamoine · 2014-06-23T09:45:40Z

@Falcon4 The problem is that RadiusOutlierRemoval should remove only outliers, not NaN values!

Falcon4 · 2014-06-24T04:24:52Z

Hi

I changed applyIndices() function like voxel_grid.hpp L61.
if cloud's is_dense flag is true process my code, other process original code.
I think that if cloud's is_dense flag correct, it work well.

But after I merged source code, Failed message indicate below.
Anything wrong?

Bye

jspricke · 2014-07-31T13:09:32Z

Hi,

the failed Travis build is an internal Travis error, nothing to worry about.

For your code, I think just testing is_dense is not the right parameter to test, as other point clouds could contain NaN values as well. I don't think there is an easy solution to check for NaN points, apart from checking all points. In that case I would propose to move this into an extra algorithm, so we can call it if we are sure to have no NaNs.

Falcon4 · 2014-07-31T23:19:43Z

Hi,

I understand your test result.
Is there anything should I do about this pull request?
If there is, please request me.

Bye

jspricke · 2014-10-01T17:57:14Z

Sorry for taking so long an please forget my comment about ``is_dense` from July. I'm fine with merging this if you can squash your commits into one. Thanks!

Falcon4 · 2014-10-17T09:58:29Z

Sorry. I missed operation (Push Revert in GitHub), so I repush a previous source code again.
And I have a problem that, I don't know how to squash my commits well.
Today, I tryid squash in local repository, but I missed it.
I think that, I squashed other's commit.
I want to roll back to "Change flow int applayFilterIndices".
Is it possible?
If you know how to do that, please teach me.
Bye

jspricke · 2014-10-17T10:04:22Z

Have a look here: http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html. Basically you need git rebase -i HEAD~6, mark the second and third as fixup and remove the lines for the last two commits. If all is fine, you can push it using the --force option. Thanks!

Falcon4 · 2014-10-20T08:39:28Z

I did a squash operation, and made three commit.
It doesn't seems to into one.
Is it ok?
If I missed, please advice me again.
Bye

jspricke · 2014-10-20T08:44:56Z

Almost ;), you have now 8 commits in your branch: https://github.com/PointCloudLibrary/pcl/pull/709/commits. I don't think you need three commits, but if, you should add reasonable commit messages. Also, do a git push -f, instead of the merge to get rid of the old commits.

radius-serach & check k --> k-search & check radius

Falcon4 · 2014-10-21T01:52:51Z

I think that I can squash my commit into one.
Please check it.
Bye

Improve RadiusOutlierRemoval's performance

jspricke · 2014-10-21T05:10:08Z

Thanks alot! And sorry for the long delay.

Falcon4 · 2014-10-21T06:16:52Z

Thank you for your advice about git too.

Falcon4 closed this May 28, 2014

Falcon4 reopened this May 28, 2014

Change search & check method.

b1cae67

radius-serach & check k --> k-search & check radius

Falcon4 force-pushed the master branch from 96fc84d to b1cae67 Compare October 21, 2014 01:49

jspricke added a commit that referenced this pull request Oct 21, 2014

Merge pull request #709 from Falcon4/master

22c9449

Improve RadiusOutlierRemoval's performance

jspricke merged commit 22c9449 into PointCloudLibrary:master Oct 21, 2014

VictorLamoine mentioned this pull request Feb 11, 2015

StatisticalOutlierRemoval hangs on point clouds with single point data. #1146

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve RadiusOutlierRemoval's performance #709

Improve RadiusOutlierRemoval's performance #709

Falcon4 commented May 28, 2014

Falcon4 commented May 28, 2014

VictorLamoine commented May 28, 2014

Falcon4 commented May 28, 2014

VictorLamoine commented May 28, 2014

Falcon4 commented May 29, 2014

VictorLamoine commented May 29, 2014

Falcon4 commented Jun 2, 2014

VictorLamoine commented Jun 4, 2014

Falcon4 commented Jun 5, 2014

taketwo commented Jun 7, 2014

Falcon4 commented Jun 9, 2014

Falcon4 commented Jun 9, 2014

VictorLamoine commented Jun 23, 2014

Falcon4 commented Jun 24, 2014

jspricke commented Jul 31, 2014

Falcon4 commented Jul 31, 2014

jspricke commented Oct 1, 2014

Falcon4 commented Oct 17, 2014

jspricke commented Oct 17, 2014

Falcon4 commented Oct 20, 2014

jspricke commented Oct 20, 2014

Falcon4 commented Oct 21, 2014

jspricke commented Oct 21, 2014

Falcon4 commented Oct 21, 2014

Improve RadiusOutlierRemoval's performance #709

Improve RadiusOutlierRemoval's performance #709

Conversation

Falcon4 commented May 28, 2014

Falcon4 commented May 28, 2014

VictorLamoine commented May 28, 2014

Falcon4 commented May 28, 2014

VictorLamoine commented May 28, 2014

Falcon4 commented May 29, 2014

VictorLamoine commented May 29, 2014

Falcon4 commented Jun 2, 2014

VictorLamoine commented Jun 4, 2014

Falcon4 commented Jun 5, 2014

taketwo commented Jun 7, 2014

Falcon4 commented Jun 9, 2014

Falcon4 commented Jun 9, 2014

VictorLamoine commented Jun 23, 2014

Falcon4 commented Jun 24, 2014

jspricke commented Jul 31, 2014

Falcon4 commented Jul 31, 2014

jspricke commented Oct 1, 2014

Falcon4 commented Oct 17, 2014

jspricke commented Oct 17, 2014

Falcon4 commented Oct 20, 2014

jspricke commented Oct 20, 2014

Falcon4 commented Oct 21, 2014

jspricke commented Oct 21, 2014

Falcon4 commented Oct 21, 2014