-
Notifications
You must be signed in to change notification settings - Fork 192
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
Adding min_age_outside_frustum parameter (solving the noisy edge problem) #183
base: noetic-devel
Are you sure you want to change the base?
Adding min_age_outside_frustum parameter (solving the noisy edge problem) #183
Conversation
As frustrating as they may be, the word is frustum, not frustrum. 😉 |
I obviously love the intent behind this PR, but I haven't yet fully grasped whether it solves the noisy edge problem or just replaces it for a different one. What about defining two frustums, one slightly bigger than the other, so that a perfectly calibrated physical sensor frustum would fall right between those two, therefore allowing a stepped treatment of voxels, that is, those that are outside, those that are inside, and those considered "on the edge" or in transition, so that the decay rates can be bridged and not basically follow a step function. Even wilder would be to somehow have the decay ramp up/down as the voxel transits between this outer and inner frustum frontiers, but that's probably a nightmare to implement. |
Damn! Pardon my french... Edited and commited |
Just try it! You ll see that it works elegantly. I set a value of 0.11s for my setup, you can also see the effect in live with rqt_reconfigure. On another note, I absolutely concur on the need of defining clearing frustum with more freedom (ie decoupling the center from the sensor origin), I can think of many use cases for that. I think I will do a PR allowing it if there is interest. The questions remains about how it should be implemented: with a tf frame ? But maybe this discussion should be on a separate thread. |
So a problem with this is that you're also checking if its
Wouldn't a better solution just be having a filter before going into the costmap to remove the points along the edges? I've done this in the past with crop filters. I've also found empirically I get better performance with this if I nudge up the frustum a few degrees from the actual camera frustum - probably I did that in response to this exact issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is me approving the code as an implementation acceptable to merge, but we still need to discuss the actual idea.
Great, I ll prepare a graphical base for discussion in the next few days when I ll get a minute |
Ok, this is my proposed base for discussion. I will used the term "cleared" instead of "having the fast decay rate of the frustum applied to" for the sake of readability. Let's represent :
Now let's examine the different possible configurations of the frustum relative to the volume covered by the sensor. Without this PRA. Matching sensor volume and frustum fovs:Here, we have the sensor fov and frustum fov matching perfectly. The well known noisy edge issue appears clearly: the red voxels are marked but not cleared.B. Frustum fov smaller than sensor volumeHere, with a frustum fov smaller than the sensor volume, we obviously increased the noisy edge issue: more voxels are marked without being cleared.C. Frustum fov bigger than the sensor volumeThis is @SteveMacenski proposed solution. The frustum clearing fov is larger than the volume covered by the sensor. In order to work, the frustum fovs needs to include the center of all voxels that can be marked by the sensor. And the maximum distance between the sensor fov edges and the center of a markable voxel is the half diagonal of the voxel ( With this configuration, we properly clean all marked voxels. However we have some problematic purple voxels. And the farther we are from the sensor origin, the more purple voxels there are. The problem with purple voxels is that we are over-clearing them. And depending on the sensor speed and the configured decay rate, some marked voxels will be purple for too long : they will never make it to the low decay rate area (outside the frustum). This is really hard or impossible to tune across all robot speeds and sensor orientations. D. Offsetted frustum (not implemented)Here, the idea, not implemented yet, is to offset the origin of the frustum from the origin of the sensor in order to enable a similar behavior as in C. but without creating more purple voxels at higher ranges. Same as in C., we properly clean all marked voxels and have less purple voxels but have still the same issue of over-clearing. With this PR: Not marking voxels whom center is not inside a frustum using the
|
This is equivalent is terms of cost of checking if each point is inside the frustum or not, and it increases the number of place where something needs to be configured. I prefer something more unified and less error prone. Plus doing it inside STVL enables the check against multiple frustums. |
True, it adds a |
It is an idea that I contemplated too, but the time a voxel will spend in this intermediate zone depends on the speed of the robot, the orientation of the sensor and which edge you are. |
Nice work with the graphics! By the way, there is something like the proposed D- Offset frustum built into the 3d lidar model (Padding). The same parameter could be added to the rgbd camera model. But it seems that at a base level, there's a need to determine whether a voxel lies on the edge, that is, we would have to check not just the sensor provided value but the 8 vertices of the voxel.so if reading's x is 3.002, we'd have to check for 3.000 and 3.005 (assuming a 5cm voxel). I wonder what the performance hit would be... |
What would a value look like for the new parameter. Is the life counted in cycles or seconds? |
I ll have to check that!
It is in second, so for me it works well with 0.11s which is just above the cycle period. |
if(!frustum_cycle) | ||
{ | ||
if (base_duration_to_decay < 0.) | ||
if (base_duration_to_decay < 0. || time_since_marking < _min_age_outside_frustum) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this wouldn't only set the threshold for newly marked points, but also points outside of the frustum that have been accelerated before, since time_since_marking
is set as the time it was marked, but is also updated by the frustum acceleration if it appeared in another iteration in the frustum.
That means that this is going to not only affect the clearing of recently marked points, but also longer-standing points that have had any level of frustum acceleration already applied. This is still overclearing.
I could make the argument that as long as the minimum age is very small, we can approximate those older points as being likely to be removed as well very soon in the next update cycle, but it does break that clear divide.
I really question if this is necessary. Going back to your problem statement: The original goal was to solve the noisy frustum edges problem
. That could be much more easily resolved for this layer and all other perception tasks you have on your robots by setting up a trivial crop filter either in STVL to knock off the edges or - more correctly - add the crop filter to your filter chain attached to your depth sensor feeds. That way you just don't have those edge points even marking at all and since they're notoriously noisy and unreliable due to poor edge calibration, you really don't want to be using that information anywhere anyway. Why not just filter that out beforehand? That seems like the best overall solution to your problem both here and in any other processing pipeline you'd ever create using the realsense cameras.
Thank you for the really extensive explanation, that helps. See the comment above though, I think you went down a bit of a rabbit hole and missed the obvious solution to your problem. The problem with noisy edges of depth cameras is incredibly common and there are common and efficient solutions to it.
That is totally not true. You have a depth image, applying a crop filter is crazy fast. You should have a light filter chain on your depth cameras as is, and if you don't, then I'm telling you that you probably should be. Your proposed approach is more error prone to overclearing voxels caught in the crossfire trying to hack around a problem with known solutions. I'd be more than happy to merge a "padding" parameter analog to that in the 3D lidar model to this, which would also help largely solve the problem you face in a much more robust way. You mention "overclearing" as a problem to the other solutions of offset / larger FOV / etc, but then you completely ignore analyzing the fact that your solution also overclears, and does so in nearly the exact same way.
As I'm sure would also be true with a padded or crop filtered FOV and have some more mathematical completeness. |
I see several independent discussions around this topic so I will try to separate them for clarity. Noisy edge issue definition
I feel that there is a misunderstanding about what I call noisy edge issue. To me it is the fact that points can be marked in the grid while not being clearable by the frustum, and so even for an ideal and perfect sensor. Proper implementation of this PR ideaI would like to separate the discussion about the proper implementation of this PR idea from its relevance (where we disagree)
I don’t understand this. The update you mention is done here right ?
So only for voxels inside the frustum. And Anyway, the idea is to remove only points that have “just” being marked outside the frustum. In order to work properly
I agree that in the current state it is error prone and needs tuning and calculation of the proper value which is not ideal. Maybe there is an automatic way to determine the proper value. Or another way of checking the “just marked” condition. Or to fallback to an additional proper Filtering inside or outside STVL ?So this is more an architectural debate.
I already have a nice GPU optimized filtering pipeline in place (noise, footprint, down sampling) and its output is used by other perception tasks (a reactive security node for instance) without issue. What you are asking is to remove perfectly valid points outside STVL in order to compensate an internal STVL issue (the quantization of the 3D space by the grid in combination with
|
In my opinion, the most complete solution would be one which avoid marking unclearable points (independently from its implementation). Cropping removes unnecessarily valid points whereas padded FOV would create overclearing issues. |
I believe you are in the pits of a "solution not the problem" mindset. I believe your real issue is that the edges of your depth images produced by your sensor is noisy, which snowballs into all these other issues. Sure, you could pad the FOV to help accelerate and deal with quantization of the voxel bins, which would also fix your problem, but lets think about what you're actually doing in that. You're processing known-noisy measurements and then trying to remove them as quickly as possible. Wouldn't it make more sense to just crop-filter them out and then go on your marry way? Padding or this modification would also help with this issue, but you could kill 2 birds with 1 efficient stone in crop-filtering out the edges. I would be OK with adding a padding feature, which would also fix this issue, but what you've implemented in this PR is probably not going to be merged. It breaks math & isn't rigorous. |
This PR adds the
min_age_outside_frustum
parameter. I am open to discussion and debate about it (and also about the name of the parameter which I am not super happy with).The original goal was to solve the noisy frustum edges problem (see for instance #141). The bigger the size (resolution) of the grid the more this problem appears, even for a perfectly calibrated frustum. More precisely, considering a non moving robot, grid voxels will be marked and not cleared under the following conditions:
This could be solved by avoiding the marking of grid voxels whose center is not inside any frustum. However this would need an additional potentially costly
IsInside
test when marking. But STVL being temporal, it is possible to get the same behavior almost for free at the nextTemporalClearAndGenerateCostmap
step where each points are alreadyIsInside
tested: if outside we simply check their age against the newmin_age_outside_frustum
param. In other words, if we find a voxel too young outside every frustum, it means that it was marked without being in a frustum.By setting
min_age_outside_frustum
just above the period of one cycle, we get the desired "don't mark voxels whose center is not part of a frustum" behavior.Bonus: by increasing this parameter we get an additional behavior which could be describe as "don't allow a voxel to stay marked outside my field of view if I did not have it under my sight for at least this amount of time".