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

Regarding the parameter num_subdivisions_per_laser_scan #627

Closed
aalapshah12297 opened this issue Dec 14, 2017 · 8 comments
Closed

Regarding the parameter num_subdivisions_per_laser_scan #627

aalapshah12297 opened this issue Dec 14, 2017 · 8 comments
Assignees

Comments

@aalapshah12297
Copy link

aalapshah12297 commented Dec 14, 2017

Hello,
I have been trying to use cartographer for indoor SLAM using a 2D LiDAR (10 scans/sec) and visual odometry (no IMU). Since my 2D LiDAR does not produce multi-echo laser scans, I set num_laser_scans = 1, num_multi_echo_laser_scans = 0 & num_subdivisions_per_laser_scan = 1, in the configuration file used in your 2D demo. I set up the remaining parameters appropriately and was able to get the node to generate maps.
However, the results were horrible and no amount of tuning seemed to fix it. After trying to debug things for almost a day, I ended up noticing in Rviz that the laser scans were being displayed 10 at a time (bundled together), and updating only once per second. I changed the value of num_subdivisions_per_laser_scan back to 10 and the issue was resolved.
Based on the documentation here, I thought that the earlier value of 1 was correct for my case. However, I now believe that this parameter refers to the number of scans per second and not what the documentation describes.
Please change the documentation/code as you see fit (or guide me if I am doing something wrong).

EDIT: The documentation mentions that this has something to do with the num_accumulated_range_data parameter, but I don't know what it does. Why is this parameter set to 10 in backpack_2d.lua? Are you combining 10 scans and then splitting them?

@cschuet cschuet self-assigned this Dec 15, 2017
@gaschler
Copy link
Contributor

Please provide more information, as noted in https://github.com/googlecartographer/cartographer_ros/blob/master/.github/ISSUE_TEMPLATE. We would need a bag file to help in this case.

The only thing I can say without seeing the data is that I'd first try to accumulate 1-5 rotations worth of scan data for scan matching, and subdivide scans into subdivisions shorter than 0.01 seconds if messages do not contain per-point time increments.
This would be num_subdivisions_per_laser_scan=10 num_accumulated_range_data=40

@cschuet
Copy link

cschuet commented Dec 20, 2017

Closing until we have more to go on.

@cschuet cschuet closed this as completed Dec 20, 2017
@aalapshah12297
Copy link
Author

aalapshah12297 commented Mar 6, 2018

I have recently continued work on my project for which I am using cartographer. I now realize that the problem was with my own limited knowledge about laser scanners. I never realized that moving scanners can lead to distorted (with respect to the real world) laser scans, though it should have been pretty obvious if I had given it enough thought.
Therefore, I did not understand the purpose of subdividing a laser scan into parts and for some reason, I assumed that cartographer was first combining multiple scans and then splitting them. I now understand that it is the actually the other way round. As for my own case, the twist part of the odometry message was incorrectly set to zero, so cartographer was not able to match the points properly as long as I didn't set num_subdivisions_per_laser_scan >= num_accumulated_range_data.
Apologies for the inconvenience. I am just leaving this comment here for anyone else who faces similar problems.

@ojura
Copy link
Contributor

ojura commented Mar 7, 2018

As for my own case, the twist part of the odometry message was incorrectly set to zero

IIRC Cartographer does not use velocities from odometry at all, so anything could be filled in there and it should not matter.

@gaschler
Copy link
Contributor

gaschler commented Mar 7, 2018

PoseExtrapolator uses the velocity from odometry (instead of the "velocity" from scan matches) if it is available https://github.com/googlecartographer/cartographer/blob/master/cartographer/mapping/pose_extrapolator.cc#L242

@ojura
Copy link
Contributor

ojura commented Mar 7, 2018

@gaschler take a look how that's computed, it's not using the twist field from odometry, but rather differentiating the pose from odometry:
https://github.com/googlecartographer/cartographer/blame/master/cartographer/mapping/pose_extrapolator.cc#L122-L131

@gaschler
Copy link
Contributor

gaschler commented Mar 7, 2018

Right, it takes the difference of odometry poses. Sorry for the confusion.

@aalapshah12297
Copy link
Author

Thanks. Actually, I recently found another possible reason why the scan-matching wasn't working very well. My laser scanner driver was reporting incorrect values within the time_increment field of the LaserScan message. I have added an issue in their GitHub repo and they corrected have the error. I will let you know if that fixes my problems.

doronhi pushed a commit to doronhi/cartographer_ros that referenced this issue Nov 27, 2018
This is in preparation of changing the data structure
for IMU data away from a deque. Needed for localization
and life-long mapping.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants