-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Hybrid A* search heavily depends on inflation layer cost_scaling_factor value #4314
Comments
How often does this happen for the choice of cost scaling factor? I.e. is what you show every single time or just on a rare occasion? Are you (reasonably) sure this doesn't also happen for values of 3/8? This is odd, I agree. When this happens, printing some data would be nice to see what these are returning in terms of cost. If you uncomment this block (and adjust the resolution/center), what's the heuristic look like? https://github.com/open-navigation/navigation2/blob/main/nav2_smac_planner/src/node_hybrid.cpp#L717-L732 |
Happens consistently, 100% reproducible. Yes, I tried a bunch of values:
as weird as it can get! I enabled the code, and I only get zeros in either case |
OK, I won't get to it this week, but this has been noted by me as something I'm going to personally investigate. If you have time / interest to look into it yourself more to make progress, that can make this move faster to a resolution. |
sorry.... I already tried and failed to find the problem... |
I can confirm that I have the exact same issue. I was (in Galactic) used to have the But since upgrading to Humble (patch 8 from February 2024) recently, I wondered why the Humble's SmacPlannerHybrid was barely (/ not at all) able to find any routes when using bigger robot footprints. I tested also with the Rolling version (downloaded/build at 4.4.2024, almost synonym to Jazzy at this point) to see with the With a footprint size of around 4m x 4m, things still work: But when increasing it to over 5m x 5m, things seem to break: But when using the 5m x 5m and reducing the The (relevant) parameters used (for 4m x 4m, commented for 5m x 5m):
This "cost scaling factor 5" behaviour happens both in the Humble version as well as in the one month old Rolling version. In Galactic, there is not such an issue. Seems that there was quite a lot of (heuristical calculation) changes to the Smac Planner in the migration from Galactic to Humble, so likely something there created this "side effect". E.g. "Replacing the wavefront heuristic with a new, and novel, heuristic dubbed the obstacle heuristic." sound like a potential source for this, but I haven't done any further/deeper investigation of the actual cause. Luckily the issue is avoidable by just a simple parameter value change (thanks to @corot for finding this affecting parameter), so that will be enough for me now 🙂 |
As an extra info, the
In the end, putting the |
That is so frecking strange, thanks @ahopsu for more follow up and detail. This definitely needs to be looked into. |
I was debugging another issue but I think I ran into this as well with SmacPlannerHybrid I was using the planner playground from @doisyg, here is my branch with the params to reproduce: https://github.com/angsa-robotics/planner_playground/tree/custom-testing-angsa |
I have been playing around with this for the last day or so. I have some observations but no reason as to why my fix works. I believe the root cause is here:
If this is changed to INSCRIBED the planner works.
This led me down a rabbit-hole trying to understand why there might have been a different condition for footprint vs radius calculations. So I started looking at the
The inscribed radius is removed a few lines later:
(Note, this could be achieved on the first line by leaving out the scale_factor * min_radius term)
The distance-from-inscribed-circle-to-obstacle is sent to the inflation layer to have a cost calculated for this distance:
But when this calculation is performed, the inscribed radius is subtracted again:
If the double inflation radius subtraction is removed, it breaks again. I think this is because if you do that the maths ends up converting from cost->distance->cost, but because costs are quantised to integer values it ends up rounding some of them down which is a problem if the original cost was 253 and it goes to 252 - invalid becomes valid. tl;dr Changing the rejection condition from |
Graphically, can you show the planning expansion footprints before and after (we have a debug topic for that now)? I want to make sure its not overly pruning all possible footprints that any part of them are in collision with the environment. Obviously if you have confined space and a big robot, parts of the robot's footprint need to be able to enter inscribed space.
Allow me to explain my thinking. Robot radius checks happens based on the robot's center cost. So the heuristic's use of However, robot footprint checks however compute the cost of the footprint edges, not the center cost, and return the single highest value ( Its not exact (since no orientation and not using the footprint), but its approximate and we use the inscribed radius, not the circumscribed radius, so its conservative. The cost used by the heuristic is thus always at least the cost that the robot would actually experience with the traversal cost function. If the robot is non-circular, it could be higher actually, but getting closer to the value (and measuring the same thing between I'll note that this feature was added while I was working on a train in the UK, so I'm just as suspicious as you now that maybe in fixing that problem I introduced another in missing a detail - but it did hugely improve the problem I was tackling at the time, so its not all bad!
I'm thinking now that this was an oversight on my part. With the context above on the heuristic needing footprint-edge costs instead of center-costs, I'd like to know what you suggest we do here instead? This treatment of the adjusted costs did really improve performance for non-circular robots non-trivially, so I think that's still the right thing to do, even if the implementation of how we accomplish that changes. I'm hoping you can review my other uses of Thanks for this dive!
Oh, if we keep that in there, please do open a PR for that. I was using the same conversion code from another part of the codebase and then modified it to remove the radius without adjusting the original term. |
Bug report
Not sure if this is a bug, but the behavior is surely puzzling, so I think it's worth reporting. I found that, for certain values of
cost_scaling_factor
parameter (I saw with 4.0 and 5.0), the Hybrid A* search becomes way less effective. Indeed looks like the obstacles heuristic somehow stops working, e,gcost_scaling_factor = 3
cost_scaling_factor = 8
cost_scaling_factor = 5 !!!!
This only happens with a non-circular footprint, so I guess it's related with how we calculate the cost here.
But I cannot imagine how changing this values to an intermediate value can have such a dramatic effect. In the turtlebot 3 little world, it still finds a valid path, but I discover this on a much larger map where it was unable to find a path in several seconds.
Required Info:
Steps to reproduce issue
nav2_bringup/params/nav2_params.yaml
with the parameter file below. It's similar to the default params except:nav2_bringup/maps/turtlebot3_world.pgm
with the attached map (with a wall bisecting the map with a single passage far from the robot)/expansions
topic on RViz (planner_server/GridBased/debug_visualizations must be true)The text was updated successfully, but these errors were encountered: