Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove warning from VoxelGrid #585

Open
schapotschnikow opened this issue Mar 21, 2014 · 13 comments
Open

Remove warning from VoxelGrid #585

schapotschnikow opened this issue Mar 21, 2014 · 13 comments
Labels
kind: question Type of issue module: filters needs: feedback Specify why not closed/merged yet

Comments

@schapotschnikow
Copy link

When I use VoxelGrid with a small grid size on a large point cloud, it sometimes gives the warning message " [pcl::VoxelGrid::applyFilter] Leaf size is too small for the input dataset. Integer indices would overflow".
VoxelGrid then returns the original point cloud.

IMO, this is nonsense. The number of points in the downsampled point cloud cannot be larger than in the original point cloud, just by definition of the voxel grid filter. And the implementation of voxel grid is such that indices never exceed the number of points in the original point cloud. Empty voxels are ignored.

My suggestion is to remove this warning. I commented out the warning in my application, and it runs just fine. The change would affect the files voxel_grid.hpp, voxel_grid.cpp, voxel_grid_label.cpp.

@taketwo
Copy link
Member

taketwo commented Mar 21, 2014

IMO, this is nonsense. The number of points in the downsampled point cloud cannot be larger than in the original point cloud, just by definition of the voxel grid filter.

I am not intimately familiar with this class, but after quickly skimming through I think the warning does not talk about point indices, but rather voxel indices. In other words, the problem is that there is not enough indices to enumerate all voxels in the volume.

@schapotschnikow
Copy link
Author

schapotschnikow commented Mar 21, 2014

I agree! The problematic line is

int idx = ijk0 * divb_mul_[0] + ijk1 * divb_mul_[1] + ijk2 * divb_mul_[2]; (1, 2, 3, 4) where idx may (but may not!) overflow.

I'd propose to store indices as integer triplets (rather than integers). This would overcome the problem, wouldn't it?!

@taketwo
Copy link
Member

taketwo commented Mar 21, 2014

Yep, at the cost of tripling the memory footprint and sorting time. Might seem unimportant, but we must be careful here as this is an essential filter that people use a lot. With a huge dataset I think the penalty this change introduces will be noticeable.

This is not a complete stopper of course. We might add a helper with two implementations (int vs. triples) and select appropriate one based on the volume size. But again, one has to be careful so that the run-time polymorphism (if used) does not introduce more penalties.

So it does not seem like a five-minute fix, but if you are willing to embark on this, go ahead.

@schapotschnikow
Copy link
Author

Well, if you are worried about the allocated size, you may take triplets of insigned __int16. It would increase the size of the index array only by a factor 1.5. This will allow downsampling a 50m long point cloud with a 1mm-grid. Currently, I am not able to downsample a 30mx5mx3m surface cloud on a 2.5-mm grid.

@taketwo
Copy link
Member

taketwo commented Mar 24, 2014

If we switch to using 16bit ints then yes, we will increase the maximum volume that we can handle. But on the other hand we will decrease the maximum possible length in each of the dimensions. Also, as I mentioned, sorting triplets will be more expensive (not exactly three times, but still).

I understand and agree that using triples of ints will increase the capabilities of the class, but we have to make sure that the existing users do not suffer from the improvement.

@jspricke
Copy link
Member

What about a template parameter to define the type? Or provide a specialized implementation.

@JamesCreasySkur
Copy link

I fixed this by replacing the pcl::VoxelGrid with a OctreePointCloudVoxelCentroid

@mtee
Copy link

mtee commented Mar 17, 2017

@JamesCreasySkur
could you please post the code, how you filter the point cloud with OctreePointCloudVoxelCentroid?
Do you simply iterate over the leaves and write them into a point cloud? Still, a code snippet would be very helpful.

@santonatos
Copy link

I have the same issue with @mtee . Have you managed to find a solution?

@YouYue123
Copy link

YouYue123 commented Mar 13, 2018

Related discussion

http://www.pcl-users.org/Filtering-huge-Pointclouds-with-Octree-and-Voxelgrid-td4030582.html

http://www.pcl-users.org/VoxelGrid-Filtering-with-Octree-td4021192.html

But I tried both. Really not working very well for me

@Duffycola
Copy link

I've only read about using triplets. What's wrong with the following proposal?

size_t idx = ijk0 * divb_mul_[0] + ijk1 * divb_mul_[1] + ijk2 * divb_mul_[2];

@stale
Copy link

stale bot commented May 19, 2020

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

@stale stale bot added the status: stale label May 19, 2020
@themightyoarfish
Copy link
Contributor

I'm getting this warning on a large point cloud even if I set leaf size to something very high, like 10m. The resulting cloud is not the same as the input, but usually cannot be rendered in pcl_viewer. I'm a bit confused how this can happen.

(pcl 1.11)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: question Type of issue module: filters needs: feedback Specify why not closed/merged yet
Projects
None yet
Development

No branches or pull requests

10 participants