Open to exposing the minimum points per voxel?

[neomanic]

Hi Steve,

I'm currently pre-processing our (noisy) depth camera data with a PCL voxel filter before handing it over to STVL, with the key difference from what you've implemented being the use of sor.setMinimumPointsNumberPerVoxel(this->thresh);

Would you be open to me exposing this setting with a voxel_min_points parameter, the same way you're already doing with voxel_size?

[neomanic]

Just realised you're using an ApproximateVoxelGrid, whereas I was using the (obsoleted and older) VoxelGrid which actually lets you set the minimum number of points. So it looks like I'll have to stick with my current preprocessing method. If you have a better suggestion would love to hear it.

[neomanic]

I went ahead and added it anyway, see: https://github.com/neomanic/spatio_temporal_voxel_layer/tree/voxel_minimum_points

I'm assuming you don't want to use the old pcl::VoxelGrid, so not asking for a pull request. But it works well and I can ditch my external filtering nodes.

[SteveMacenski]

Hi, It looks like you found a place in the code where I set the thresh, if that value isnt exposed you’re welcome to open a PR implementing it! I would welcome it.

Why do you say the approximate voxel grid is better? They’re different for different uses. I wouldn’t think that the exact voxel grid is “obsolete”

I’m currently in Korea for work (on my phone) so I cant review a branch but if you think that parameter is useful I’m open to merging it.

[neomanic]

Happy to wait to discuss further until you're back.

I think it's useful to add voxel_min_points for anyone who has a noisy depth sensor. For instance, we use our depth cameras for obstacle avoidance, but were finding that a single false pixel was stopping us in our tracks far too often, and we settled on a minimum of 5 points required per voxel.

You're right, VoxelGrid isn't deprecated in PCL. (I was apparently reading the libpointmatcher documentation instead, where it is deprecated. 🤦🏼‍♂️)

ApproximateVoxelGrid does not have the setMinimumPointsNumberPerVoxel() method, whereas VoxelGrid does. You're currently using the former.

The branch I linked above adds a voxel_min_points config parameter to each source, and changes the ApproximateVoxelGrid to be a VoxelGrid filter instead.

Perhaps I should check the voxel_min_points parameter first, and use an ApproximateVoxelGrid if it is == 1, and VoxelGrid if it's > 1. That way it would have the exact behaviour as it does now if the parameter is not used. Thoughts?

[SteveMacenski]

Ah, the main reason I chose approximate I think was the docs (or assumption?) saying that Approximate was less CPU overhead and given that we bin the voxels to ~2-5cm, its not like the "exact" is really going to be much improvement or noticeable.

If you don't see any CPU growth, I don't see a problem with changing to the non-approximate one and expose that parameter. I agree that that is useful capability. Or we could have it do as you suggest and optionally change over to it if the parameter is a real value.

[neomanic]

Thanks. At the moment I'm playing around with doing all the filtering externally, since we've reconfigured our robot model and now I need to take imaged parts of the robot out of the input point cloud before feeding it into STVL. But I'll come back to this in a bit.

[SteveMacenski]

Keep in mind there's a clear-within-footprint option in navigation. If you don't want to filter out your cloud yourself for conflicts of the robot, but obviously there's some drawbacks of that. External filtering is good, there's very few sensors I've ever worked with that I could just plug into my world model without any conditioning.

[neomanic]

Yep, should have been clearer, sorry.

The issue is that if there's voxels in the STVL that are within the footprint of the robot, these are occluded by that same portion of the robot when we move forward, and so they aren't removed from the STVL until the decay time kills them off— leaving a stream of obstacles behind the robot.

I was going to post a comment about adding this functionality into STVL, but once diving into the source I worked out they'd have to be removed from the buffers, so it's better off just doing it beforehand with pre-processing.

Once I'm done with this filter, I'll post it up and post a comment here, so anyone needing this prefilter functionality can use it. Feel free to close this issue.

[SteveMacenski]

Look here: https://github.com/ros-planning/navigation/blob/5fe08786d9058ea4dfe5b3292a6622efbb48a153/costmap_2d/plugins/obstacle_layer.cpp#L240

https://github.com/SteveMacenski/spatio_temporal_voxel_layer/blob/melodic-devel/src/spatio_temporal_voxel_layer.cpp#L589

You can clear the footprint under the robot of all cost with this parameter, its done for you

[neomanic]

I'm familiar with those options and have them enabled. I'll try to explain myself better.

Screenshots below have the STVL voxel layer displayed as that gridded point cloud, and the costmap underneath.

When the robot is still. Note the voxels inside the footprint, which are excluded from the costmap due to the footprint. So far; so good.

We start moving. Because of the trolley, the view backwards is occluded, and so the voxels aren't cleared from the layer.

The (non-decayed) voxels are now off the back of the robot, and are now added to the costmap. This is where it's gone wrong.

If we wait for the decay time, then it clears.

Does this make sense? Am I right in thinking that the best solution is preprocessing to throw away any points in the depth cloud that are within the footprint?

[SteveMacenski]

mhm, that appears to be a bug then on STVL's end. What should happen is that when they're cleared in the costmap grid to not appear in the costmap, they should also be cleared from the OpenVDB grid, which is doesn't appear to be doing if they start appearing again when the robot leaves that voxel area.

Its clearing from the costmap_2d layer but not the underlying openVDB grid

Thanks for the (shockingly) high quality images to help identify the issue, that really illustrates it

[neomanic]

Just screencaps from a video; should have done that from the get-go. Good to hear you think it's a bug and not just me going mad.

Here's the gist with my preprocessing, if people run into the same issues, there's some solutions there: https://gist.github.com/neomanic/00de3fe955d0c53b223fcba0b7d90e2d

[SteveMacenski]

You should be able to patch when we clear footprint to clear the points in the underlying openVDB grid. I will eventually get to it now that you've pointed it out but I'm under the gun on a few other projects at the moment

[neomanic]

No worries— no pressure from me. Thanks for the help.

[SteveMacenski]

As an FYI if this interests you, this is where it would need to go (and for my notes later): https://github.com/SteveMacenski/spatio_temporal_voxel_layer/blob/melodic-devel/src/spatio_temporal_voxel_layer.cpp#L642

In this we're setting clear costs in the polygon of the footprint, we also need to do that same thing for the openVDB grid.

[nickvaras]

BTW, I very recently got a PR approved for the Melodic pcl_ros VoxelGrid that exposes the minimum points per Voxel. Just thought to let you know.

[SteveMacenski]

Nice! Feel free to link it here too so we can track it

[nickvaras]

https://github.com/ros-perception/perception_pcl/pull/243

[SteveMacenski]

I think we started to have 2 conversations here

• @neomanic I found a library call that should make clearing the footprint in the grid easy. We can combine 2 grids together in a custom way very quickly, so we can make a grid of just the footprint all deactivated and then deactivate the voxels in the real grid in the footprint each iteration with combineExtended(). Other option is in the clearing loop where we visit the cells, we query if in footprint and then deactivate it if the param is set. This is easier to implement but not as clean architecturally (or as cool). But frankly if you’re going to do either I’ll merge both. I have no back of the envelope proof that merging the trees will be any faster and we’re already iterating through the tree

• @nickvaras and @neomanic if that’s been merged feel free to submit PRs to add the utility for min points. We can close this ticket then and open a new one on the footprint clearing bug

[BriceRenaudeau]

Hi, It would be great if you could make a pull request exposing the min_points_per_voxel parameter. Thanks

[SteveMacenski]

Any updates guys?

[connoranderson]

@SteveMacenski it looks like this change is already in, but a new build needs to be triggered for older distros (it's not included as of today in 1.7.0). For now you can build from source to get the feature.

[SteveMacenski]

I'll accept a PR and look to releasing PCL perception

[SteveMacenski]

@connoranderson @nickvaras I just cut a new 1.7.1 release to include this in melodic. Let me know if there's anything else, closing ticket. @neomanic

Sorry that took so long.