Adding LoopClosure checker to serialize a posegraph correctly

[abylikhsanov]

I am not sure that I did that correctly as I had a chance to look at karto code only today. I tested on my device and it did not allow to serialize the posegraph at the beginning and it was okay afterwards.

[SteveMacenski]

Just to verify before I review: are you saying this 100% fixed your error?

[abylikhsanov]

Yes, so far it always worked

[SteveMacenski]

Concept is good, lets rename all the variables though to be clear.

I’m thinking something like optimizer_cached.

Can you add a sentence or two in the readme about “hey, it won’t serialize until a loop closure attempt has been triggered for performance caching reasons”

[SteveMacenski]

Hi, any updates?

[abylikhsanov]

Hey sorry for a such delay, had a crazy week. I have updated the code now

[SteveMacenski]

LGTM

[SteveMacenski]

I'm debating right now what to do about this -- since we change the serialization signature, I'm not sure if this will corrupt everyone's files.

Have you tried to deserialize a file that was generated before this change into this now new version?

I see there's versioning: https://stackoverflow.com/questions/23230369/boost-class-serialization-change-in-member-types such that we could have new files set to this version, but if missing, then old files will just ignore that field with the if(right_version){ar & variable}

[abylikhsanov]

But to use the version, how would you add a version? As it also needs to be serialized, isn't?

Yes, I checked again and it will crash if you load the older serialised map

[SteveMacenski]

Mhm. Alright here's what we can do (https://www.boost.org/doc/libs/1_53_0/libs/serialization/doc/tutorial.html search for string if(version > 0)):

0. Add BOOST_CLASS_VERSION(class_name, 2.1.1) where 2.1.1 is the current melodic package.xml version for all classes. Recompile and check that it's been taken in the serialize function. (just print version)
1. Version is defaulted to 0 when none given, so we check version and if(version > 0) {serialize/deserialize that variable}. Older files should then work fine. Test and make sure both old and new work.
2. for all new serialization/deserialization additions, we need to keep the version up to date. (that one's on me)

Does that make sense?

[abylikhsanov]

Yes, seems straightforward. I will do that tomorrow

[abylikhsanov]

I have a question. In Mapper.h file, there are two friend classes which do the serialization. From my understanding, I have to stick to the MapperGraph serialization as it just makes sense. Should I only check/serialize in the MapperGraph?

[SteveMacenski]

I don't understand the question.

[SteveMacenski]

Any update?