-
Notifications
You must be signed in to change notification settings - Fork 767
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
testSmartProjectionFactor unit test fails with march=native #590
Comments
This might even be a bug in the older gcc version... I'll try to run all tests with optimize native ON on an u20.04. PS: GTSAM_BUILD_WITH_MARCH_NATIVE might be OFF by default, it's a recurrent topic, but in no case should that flag lead to a failed test, only to problems in 3rd party user programs if built with different combinations of eigen-version and/or optimize-native. |
A few more data points:
|
Some tests:
Crashes occur at:
Rebuilding with optimize native=OFF for clang-10 also segfault's, so the "optimize native" doesn't seem to be the problem here (?). Rebuilding with system Eigen also leads to segfault. This seems like either a bug of clang (or old GCC) + boost serialization, right? We are not alone: |
What version of Boost are y'all using? I believe this is a boost bug from before version 1.67, because I had faced a serialization segfault issue in the recent past. The basic problem is that the older boost serialization code would do a shallow serialization, so when de-serializing any complex factor, the test code would try to access something that wasn't initialized, hence segfault. I am using |
Just tried with both @chrisbeall please do try running the tests with Boost 1.67+. |
1.65.1. I'll try 1.67+. Any painless way to install it w/o building boost from sources for u18.04? |
PS: If boost is confirmed to be the cause, at least a note should be added to the README. |
This link has a custom PPA: https://asciinema.org/a/199344 |
@chrisbeall @jlblancoc any updates? I apologize for the pushes, but there is some stuff I wish to investigate so fixing this niggling issues should help my cause. |
Gentle reminder. If this is indeed a boost version issue, I'd like to update the CMake and the docs to reflect this ASAP. 🙂 |
Maybe we should consider (seriously) getting rid of boost, as the biggest boost dependency has been eliminated (which coincidentally is the |
i would like to see boost gone, since it is a large system dependency i really dont think we need, or even want |
That's not quite correct. We use boost for the OptionalJacobian amongst other things. Also I don't quite see @johnwlambert's point about it being a big system dependency that we don't need. You install boost once and forget about it, plus it can be used for other C++ projects and scripts, e.g. for parsing command line arguments. |
Anyway, whether or not boost is needed is tangential to the point of this PR. @jlblancoc @chrisbeall can y'all let me know if upgrading boost fixed this issue for you? |
On a cluster where one does not have sudo access, boost installation can be difficult to get right using conda -- I've seen this recently on Skynet. |
Well that's not a Boost problem, that's a lazy admin problem. Moreover the fix in that case is incredibly simple: install Boost to a location you have access to and just add that location to the Cmake and ldconfig env variables, and voila! |
FYI that's how I manage multiple versions of boost on my system, no issues. |
Boost is a burden for us because boost is a source of uncertainty - I think bugs in boost reflected on GTSAM have cost me 100+ work-hours to debug in the previous two years. I would say if we are targeting Ubuntu 18.04+ (which = GCC 7.4), then we should have most of the Boost functionalities in |
Anyway I agree this is tangential to this issue so let's move the discussion elsewhere. |
@ProfFan :
If it's acceptable to impose the minimum standard C++17 to users, then it would be a really easy port on
Thanks for the PPA. I had to remove ROS to allow apt to install boost 1.67, but it's ok. However, I couldn't get cmake to be happy with this 1.67.0 version, I'll try to continue investigating later, but just in case someone knows the solution, here's the issue: cmake -DBoost_ADDITIONAL_VERSIONS="1.67.0" ..
...
-- Could NOT find Boost (missing: serialization system filesystem thread program_options date_time timer chrono regex) (found suitable version "1.67.0", minimum required is "1.58")
CMake Error at cmake/HandleBoost.cmake:33 (message):
Missing required Boost components >= v1.58, please install/upgrade Boost or
configure your search paths.
Call Stack (most recent call first):
CMakeLists.txt:41 (include) |
Moving to c++17 and removing boost, if we decide to do that, is not going to happen until the next major version change (5.X.Y), so let's not discuss too much. |
u18.04 (amd64) + clang-10 + ...
So it seems that minimum Boost version should be 1.67, at least for clang. cc: @varunagrawal Should you perhaps update the README? |
@chrisbeall encountered this issue for gcc as well, so if he can confirm that upgrading Boost to 1.67+ works, we can close this. @jlblancoc created the PR! #681 |
Quick update: I tried this on an Ubuntu 16.04 docker image with GCC 5.4.0 and Boost 1.58 and I cannot reproduce the issue. @chrisbeall am I missing something? |
Does #681 imply that this issue could be closed now? |
I believe that to be the case. |
Closing this due to lack of movement and since we've added it to the list of known issues. |
Description
testSmartProjectionFactor
fails on Ubuntu 16.04, gcc 5.4.0 with default configuration. The default configuration usesmarch=native
, which is not exercised by CI. Test passes withGTSAM_BUILD_WITH_MARCH_NATIVE=OFF
.I'll try to look into this myself later, but wanted to make a report.
Steps to reproduce
Get latest develop, configure,
make check
.Expected behavior
Test passes.
Environment
Ubuntu 16.04, gcc 5.4.0
The text was updated successfully, but these errors were encountered: