-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Save/Load Atlas Functionality #310
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
Conversation
|
|
||
| #include <boost/serialization/base_object.hpp> | ||
| #include <boost/serialization/map.hpp> | ||
| #include <boost/serialization/set.hpp> |
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.
These are important
|
|
||
| //Save/Load all objects | ||
| ar & mspMaps; | ||
| ar & mvpBackupMaps; |
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.
is this necessary?
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.
Now that you mention it, serializing mspMaps should not be required given that these are backed up in PreSave and rebuilt in PostLoad. I have not yet tested without that line if it works. If it's the case I'll make the change ;) Thanks !
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.
you are right, either mspMaps or mvpBackupMaps is ok. https://github.com/UZ-SLAMLab/ORB_SLAM3/pull/310/files#r627846281
| include/MLPnPsolver.h | ||
| include/TwoViewReconstruction.h | ||
| include/Config.h | ||
| include/serialize_tuple.h |
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.
why not directly use boost library? It works with me.
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.
When I serialized large maps, I had a bug when recovering the MapPoints's observations from the backup. Serializing mObservations directly solved the problem. However those observations contain a tuples. If I'm not mistaken, by default, there is no tuple serialization in Boost. That's why this is required.
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.
I didn't meet this kind of problem. Will your program break down with red error? Recover from backup is ok from my side.
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.
not sure if this is helpful. https://github.com/UZ-SLAMLab/ORB_SLAM3/pull/310/files#r627844802
But I think add serialize_tuple.h is also a good option:)
| else | ||
| { | ||
| EraseObservation(pKFi); | ||
| } |
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.
I comment out line 605-608 when recover mObservations from backup, probably this the bug
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.
I comment out line 605-608 when recover mObservations from backup, probably this the bug
Hello,
Could we add wechat (william_hjx)? I have few questions about the commented lines. Thx!
| mspMaps.clear(); | ||
| unsigned long int numKF = 0, numMP = 0; | ||
| map<long unsigned int, KeyFrame*> mpAllKeyFrameId; | ||
| for(Map* pMi : mvpBackupMaps) |
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.
I use for(Map* pMi : mspMaps) here directly, since mspMaps is already stored and mvpBackupMaps may be not necessary
|
@neo-knight-td why are you comment all of |
|
Good job! I have tested it, only sometimes when loading a map it may occur core dump, but works well after loading the map again in stereo mode. Stereo_Inertial not working when loading map, just core dump. |
|
I was using only monocular to run this case but failed with my rosbag...Map can be saved and reloaded but when I changed into relocalization mode, frames will disappear in screen. So everything got blank ...Are there some variables I didn't save or load? |
| SetPose(Tcw); | ||
|
|
||
| /* | ||
| // Reference reconstruction |
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.
Why comment on these lines?
| else | ||
| { | ||
| EraseObservation(pKFi); | ||
| } |
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.
I comment out line 605-608 when recover mObservations from backup, probably this the bug
Hello,
Could we add wechat (william_hjx)? I have few questions about the commented lines. Thx!
@PattyLiang As far as I was concerned, I did not need the inertial variables to be serialized. Therefore I commented those lines. |
@phdsky Thank you ! With which maps did you encounter problems at loading ? Maps from which sequences ? Stereo or Mono ? |
@Jiaxin8122 What do you mean by "change in relocalization mode" ? You trigger the change manually ? On which sequences do you experience such errors ? |
Yes, I use the bottom from GUI to begin the LocalizationMode. The save and load function in my case looks nice. But when I wanna load the saved map to relocalize, this error happened. Also, just curious why you commented the save map point id and observation. |
I never tried the LocalizationMode only (even from non loaded sequences). I don't exactly know what it does in the code when this button is pressed. I will need to dig a bit in order to answer your question. Also, regarding the commented lines, it's been quite a while since I did this so I don't remember the exact reason but certainly this was to avoid other core dumps. I come back to you when I have more answers to your questions ;) |
That's really helpful. Thx a lot! |
|
@Jiaxin8122 What did you use as sequences in Monocular ? I tested on MH01 from EuRoC, played with the Localization Mode button with a loaded sequence but I have no "frame disappearing" or whatsoever. Could you provide some screenshots ? |
|
Code Update : The code for Monocular is integrated. The example file is adapted. Also, unserialized MspMaps (was not needed as there is a backup of it). Code for inertial will come later. |
|
Sorry for the delay, quite busy at the moment :/ Unfortunately I could not replicate your bug. I added the code I use for Monocular. Try to use use it and tell me what you get with it. I tried switching on and off the Localization Mode button but did not ended with such blancks or whatsoever :/ Could you provide your code/bagfile ? ps : Regarding the Localization Mode, the only thing it does is turning off the Local Mapping thread ;) |
Hi, thx for your answer. I have solved my problem because there is a bug in the tracking thread. Thanks for your helping again! |
|
Hi, I am testing your code and found a problem which can be replicated following the next steps:
After some investigation, I´ve found out that the problem occurs here. Apparently, one (or some) of the pMPi pointers are invalid. Therefore, GetObservations().size() fails. Do you know where the problem could be? |
After some more debugging I have found out the problem is here. Apparently at one given iteration, executing EraseObservation() corrupts somehow mspMapPoints. Hence, at the next step pMPi points to an invalid location. |
Hi guy, thx for your contribution about the project. I have been testing ORB_SLAM3 mono_imu as saving map for weeks. But it often seg fault. |
For Saving an Loading an Atlas session. This solves the serialization bug of version V.0.3 (from which most of the code is borrowed). Map merging is also solved. An example script is made available.