iris-ua / iris_lama_ros

LaMa on ROS
BSD 3-Clause "New" or "Revised" License
208 stars 59 forks source link

Reconstructing a map in offline mode might produce incorrect maps #23

Open jcmonteiro opened 3 years ago

jcmonteiro commented 3 years ago

I have noticed that the map that I obtain when running the algorithm in the robot in real-time is always better than the one that I get from replaying the ROS bag and running the offline node, e.g., see #21.

I concluded that this is because the messages are published too fast by the ReplayRosbag function for the algorithm to consume them. Increasing the queue as suggested in #21 and enabling TCP_NODELAY as suggested in #22 help mitigate the issue, but it still doesn't go away. To check that this inference is right, I have reduced the rate from 5 kHz to 30 Hz https://github.com/iris-ua/iris_lama_ros/blob/594b91957ff12dcb0c3bf46fbee7023865da9c12/src/offline_replay.cpp#L79 matching the rate on the real system, and producing maps that are exactly the same and exactly the same as the one generated in real-time.

My suggestion then is to change the offline node to use https://github.com/iris-ua/iris_lama as library directly, without publishing and subscribing to topics, thus ensuring that no messages will be lost. I can maybe do this in a week or so if I get some spare time.

jcmonteiro commented 3 years ago

Another possible solution to mitigate the issue would be to convert the node to a ROS nodelet.

eupedrosa commented 3 years ago

Thank you for addressing this issue.

My suggestion then is to change the offline node to use https://github.com/iris-ua/iris_lama as library directly, without publishing and subscribing to topics, thus ensuring that no messages will be lost. I can maybe do this in a week or so if I get some spare time.

This is what I had while I was developing LaMa, it was ROS free.

jcmonteiro commented 3 years ago

Well, if you had it and can make a quick rollback, maybe that would be better. The way it is, the algorithm is under-performing, and it's not even its fault. Kudos on the implementation btw; it's so easy to go from the papers/theory to the code.

eupedrosa commented 3 years ago

The offline version that I have only reads CARMEN log files. And you are correct, the algorithm is under-performing and that is not the desirable outcome.

Your ROS nodelet seems to be a sensible sugestion, but before going this path, I think it is possible to solve the problem without adding more "complexity".

Thank you for the kudos :smiley:

jcmonteiro commented 3 years ago

Well, converting from rosbag to CARMEN should be easy enough. I'd say that's the way to go if you can make the offline version using CARMEN files available 😬.

eupedrosa commented 3 years ago

@jcmonteiro, I think I found a possible solution.

The idea is to replace https://github.com/iris-ua/iris_lama_ros/blob/fc35175a6d1279a5a8b3161724014defcb74beda/src/pf_slam2d_ros.cpp#L101 with

tf_ = new tf::TransformListener(nh_, ros::Duration(10), false);

and also delete https://github.com/iris-ua/iris_lama_ros/blob/fc35175a6d1279a5a8b3161724014defcb74beda/src/offline_replay.cpp#L108

The tf::TransformBroadcaster(), by default, creates it own spin thread. This may be the reason why tfs are lost.

I tried this with only one dataset and it worked. But it would be great if you could verify with your dataset.

eupedrosa commented 3 years ago

@jcmonteiro, Forget about it. I guess I was wrong! It changes nothing :disappointed:

jcmonteiro commented 3 years ago

Hahah I was ready to test it out! Going back to the CARMEN thread, the solution does not sound very elegant imho... Maybe calling the API directly after extracting the messages is the way to go.