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

global_mapping_pose_graph fast forwards too much when standing still at the start #113

Open
sgvandijk opened this issue Nov 18, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@sgvandijk
Copy link

Describe the bug
Thanks very much for making this great project available! I have tried out global_mapping_pose_graph since in some cases it seems to give better loop closures for me when the travel distance between those closures are very far (>1km).

However, in some cases it fails to find any closures, and I tracked it down to the fast forwarding logic in find_loop_condidates. Specifically here:

const int step = 0.8 * direct_dist / std::min(travel_dist_avg, 100.0);

When the travel distance between the first submaps is very small, in my case in the order of centimetres, and the direct distance is large, then the fast forward step is very big. In my case this can be in the 1000s, and it always fast forwards past submaps.size().

I removed the fast forward logic, and then things worked well for me.

Expected behavior
I am not sure what the ideal behavior would. Setting an upper limit on step size may make sense, but that would reduce the usefulness of the fast forwarding in cases where big steps are the correct thing to do. Alternatively a lower limit on travel_dist_avg could help, but a reasonable value of that may differ case by case.

@sgvandijk sgvandijk added the bug Something isn't working label Nov 18, 2024
@koide3
Copy link
Owner

koide3 commented Nov 24, 2024

Thanks for reporting the issue. I'll add a patch to limit the maximum fast forward steps as a quick fix. I'm also thinking of implementing a better proximate keyframe selection (maybe with a dynamic KdTree).

@koide3 koide3 pinned this issue Nov 24, 2024
@koide3
Copy link
Owner

koide3 commented Nov 26, 2024

I just merged a patch to limit the fast forward steps up to 10 #120.

@koide3 koide3 unpinned this issue Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants