Skip to content
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.

Sign up for GitHub

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

Jump to bottom

FTBFS when using TBB #53

Closed
jlblancoc opened this issue Jun 10, 2019 · 11 comments
Closed

FTBFS when using TBB #53

jlblancoc opened this issue Jun 10, 2019 · 11 comments

Comments

@jlblancoc
Copy link
Member

It seems there are now build errors when TBB is enabled:

gtsam/linear/VectorValues.h:200:22: error: no member named 'emplace' in 'gtsam::ConcurrentMap<unsigned long, Eigen::Matrix<double, -1, 1, 0, -1, 1> >'
      return values_.emplace(j, value); 
             ~~~~~~~ ^

due to the selection of CONCURRENT_MAP_BASE.

What's the preferred solution here? For now, disabling TBB is the only workaround.
@dellaert
Copy link
Member

Hmm. Fix might be as simple as values_.insert(make_pair(j,value)) ?

@dellaert
Copy link
Member

Maybe guarded by TBB flag, as emplace is more efficient I think

@chrisbeall
Copy link
Member

I bumped up the minimum required version of TBB in #23 because someone already reported this issue in #19. What version of TBB do you have?

@jlblancoc
Copy link
Member Author

jlblancoc commented Jun 10, 2019 •
edited
Loading

Apparently, this is my version but cannot find any "4.x" anything on that page (?).
But this file /usr/lib/x86_64-linux-gnu/libtbb.so.2 seems to mean Bionic comes with version 2? Or am I totally missing some point?

@dellaert
Copy link
Member

Hmmm, maybe if the fix above works there is no need to bump?

@chrisbeall
Copy link
Member

No, you don't have version 2. Ubuntu 16.04 has default version 4.4, and Ubuntu 18.04 has version 2017+. The suffix on libraries often has nothing to do with version numbers.

@jlblancoc
Copy link
Member Author

Hmm... should then I apply this proposed patch if using TBB?
@chrisbeall Does this fail for you too when building with TBB?

@chrisbeall
Copy link
Member

Building with TBB works fine for me on Ubuntu 16.04 with TBB 4.4, and on Mac with TBB 2019-U5. I can't think of any reason why this would not work on Ubuntu 18.04 with TBB 2017-U7, but I've not yet tried.

Here you can see that TBB changed their versioning scheme. After 4.4.x they switched to using year + update #.
The only issue I ran into is the testDoglegOptimizer test failure, which I reported in issue #41

@dellaert
Copy link
Member

I would try the patch, and if compiles, PR it. But keep emplace in non-TBB path?

@chrisbeall
Copy link
Member

chrisbeall commented Jun 11, 2019 •
edited
Loading

I just tried from scratch on Ubuntu 18.04 with TBB 2017-U7 on latest develop (4e19ba4) , and everything compiled. The version of TBB you have clearly supports move semantics, so there must be some misconfiguration or bug somewhere. Let's find the root cause, please?

@jlblancoc
Copy link
Member Author

Also updated to the latest develop commit and it builds ok.
Closing, sorry for the noise!

varunagrawal added a commit that referenced this issue Mar 24, 2021
@varunagrawal
Squashed 'wrap/' changes from aae9b4605..3eff76f60
58a9622 
3eff76f60 Merge pull request #53 from borglab/feature/refactor
13215dfa7 Merge pull request #52 from borglab/fix/tests
696913b11 install setuptools
9523328ba Merge branch 'master' into fix/tests
7c630b361 some more cleanup
656993a71 cleaned up  Typename
a16f6f38e move qualified and basis type outside to their own class scope
72ead8bd7 Merge pull request #51 from borglab/fix/test-interface-parser
6deefd4fc added tests for interface_parser
50d490a85 Merge pull request #50 from borglab/feature/refs-all-types
be4511290 updated docs for BasisType
0e80b0d8c update MATLAB tests
0015d7397 added support for shared pointer and ref for basis types
86d2158f1 remove std::string from list of Basis types
94f928441 ignore code coverage reports
2033dd345 replace prints with log.debug statements
ae98091b3 fix deprecation in doc tests
13a2f66c4 Merge pull request #46 from borglab/feature/new-shared-pointer
3c7d85865 updated docs
6d7897088 use @ for raw pointer, go back to * for shared pointer
1d6194c57 updated matlab wrapper to handle both raw and shared pointers
1448f6924 fix some failing tests
2ab1dae32 Merge branch 'master' into feature/new-shared-pointer
96f8a56bd Merge pull request #47 from borglab/fix/ci
6003203f3 run CI only for pull requests
a8f29ead1 fix the python version yml key
fcae17227 check if directory exists when testing
f592f08c9 explicit pip3 so that we don't use Python2
d49c2f3c2 correct call for pip
dfe360526 fix the CI
149b7c523 docs for templated functions
f2189acc6 support typedefing for templated functions
965458a2b added test for templated functions
eaff6e6ab made is_const common for all types
3d9c70b32 added tests and cleaned up a bit
010b89626 support for simple pointers on basis types
6b98fd80c new syntax for shared_ptr
ff7ad0b78 support for templated functions
a1a443c8d Merge pull request #43 from borglab/fix/cmake-and-matlab
2f3a055e4 remove accidentally committed file
770d055e2 set proper paths for cmake and eschew relative paths
773d01ae1 fix bug in matlab wrapper
721ef740f Merge pull request #41 from borglab/feature/type-hints
67aac9758 minor refactor of CI yml
e6a63ae0c fix all mypy issues
a3aaa3e7c remove a lot of linter issues from matlab_wrapper
a96db522f static typing for interface_parser

git-subtree-dir: wrap
git-subtree-split: 3eff76f604b5ba9e71cf4947654e288142ed7a94
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jlblancoc @chrisbeall @dellaert