Remove boost from TableFactor and added guards to testSerializationSlam#1552
Conversation
varunagrawal
left a comment
varunagrawal
left a comment
There was a problem hiding this comment.
Looks good but I believe the guard in the test file should not be needed.
| #include <gtsam/base/serializationTestHelpers.h> | ||
| #include <gtsam/base/std_optional_serialization.h> | ||
|
|
||
| #ifdef GTSAM_USE_BOOST_FEATURES |
There was a problem hiding this comment.
I think this can be removed since we exclude this test file using CMake if boost serialization is disabled.
Ref: tests/CMakeLists.txt
Does this test file still get compiled even with the boost serialization flag set to OFF?
There was a problem hiding this comment.
Yes, with both USE_BOOST_FEATURES and ENABLE_BOOST_SERIALIZATION set to OFF, it still attempts to compile this test.
I think I see the problem. the CMakeLists.txt tries to exclude "testSerializationSLAM.cpp" (with SLAM in all caps) instead of "testSerializationSlam.cpp"
There was a problem hiding this comment.
That seems plausible. I can't reproduce the issue on my end but that may be because of differences in CMake versions or the platform (I'm using MacOS).
Can you please update the CMakeLists.txt file? Then we can remove this guard.
There was a problem hiding this comment.
OK, I have update the CMakeLists.txt file and removed the guard, and now it works on my end.
| cout << " f["; | ||
| for (auto&& key : keys()) | ||
| cout << boost::format(" (%1%,%2%),") % formatter(key) % cardinality(key); | ||
| cout << " (" << formatter(key) << "," << cardinality(key) << "),"; |
There was a problem hiding this comment.
This looks great. I can verify the printing is the same as boost.
2 participants
This PR removes the dependence on boost from the recently added
TableFactor, and adds some guards totestSerializationSlamif boost is disabled. These were currently the only two things preventing gtsam from building and passing all unit tests without boost.I attempted to maintain the style of the output in TableFactor, but I didn't explicitly test with boost to compare.