From 366b2485c1d53c21d887a08357d6383c4dfe8b21 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 21:00:22 -0400 Subject: [PATCH 01/13] Fixed warnings flagged by Ubuntu g++ --- gtsam/3rdparty/CCOLAMD/Source/ccolamd.c | 11 ++-- gtsam/3rdparty/metis/GKlib/csr.c | 32 ++++++----- gtsam/3rdparty/metis/GKlib/getopt.c | 71 +++++++++++++------------ gtsam/inference/BayesTree-inst.h | 14 ++--- 4 files changed, 66 insertions(+), 62 deletions(-) diff --git a/gtsam/3rdparty/CCOLAMD/Source/ccolamd.c b/gtsam/3rdparty/CCOLAMD/Source/ccolamd.c index 9a08e3ea8..d5e8da2f0 100644 --- a/gtsam/3rdparty/CCOLAMD/Source/ccolamd.c +++ b/gtsam/3rdparty/CCOLAMD/Source/ccolamd.c @@ -1560,8 +1560,9 @@ PUBLIC Int CCOLAMD_2 /* returns TRUE if successful, FALSE otherwise */ Int *dead_cols ; Int set1 ; Int set2 ; +#ifndef NDEBUG Int cs ; - +#endif int ok ; #ifndef NDEBUG @@ -1909,8 +1910,10 @@ PUBLIC Int CCOLAMD_2 /* returns TRUE if successful, FALSE otherwise */ p [k] = col ; ASSERT (A [col] == EMPTY) ; - cs = CMEMBER (col) ; +#ifndef NDEBUG + cs = CMEMBER (col) ; ASSERT (k >= cset_start [cs] && k < cset_start [cs+1]) ; +#endif A [col] = k ; k++ ; @@ -1926,11 +1929,11 @@ PUBLIC Int CCOLAMD_2 /* returns TRUE if successful, FALSE otherwise */ if (A [col] == EMPTY) { k = Col [col].shared2.order ; - cs = CMEMBER (col) ; #ifndef NDEBUG + cs = CMEMBER (col) ; dead_cols [cs]-- ; -#endif ASSERT (k >= cset_start [cs] && k < cset_start [cs+1]) ; +#endif DEBUG1 (("ccolamd output ordering: k "ID" col "ID " (dense or null col)\n", k, col)) ; p [k] = col ; diff --git a/gtsam/3rdparty/metis/GKlib/csr.c b/gtsam/3rdparty/metis/GKlib/csr.c index a19d793bd..57934ee9d 100644 --- a/gtsam/3rdparty/metis/GKlib/csr.c +++ b/gtsam/3rdparty/metis/GKlib/csr.c @@ -1323,29 +1323,27 @@ void gk_csr_Normalize(gk_csr_t *mat, int what, int norm) ssize_t *ptr; float *val, sum; - if (what&GK_CSR_ROW && mat->rowval) { - n = mat->nrows; + if (what & GK_CSR_ROW && mat->rowval) { + n = mat->nrows; ptr = mat->rowptr; val = mat->rowval; - #pragma omp parallel if (ptr[n] > OMPMINOPS) +#pragma omp parallel if (ptr[n] > OMPMINOPS) { - #pragma omp for private(j,sum) schedule(static) - for (i=0; i 0 */ +#pragma omp for private(j, sum) schedule(static) + for (i = 0; i < n; i++) { + for (sum = 0.0, j = ptr[i]; j < ptr[i + 1]; j++) { + if (norm == 2) + sum += val[j] * val[j]; + else if (norm == 1) + sum += val[j]; /* assume val[j] > 0 */ } if (sum > 0) { - if (norm == 2) - sum=1.0/sqrt(sum); - else if (norm == 1) - sum=1.0/sum; - for (j=ptr[i]; jconditional_->frontals()) { - if(!first) parent += ","; first = false; + for (Key index : clique->conditional_->frontals()) { + if (!first) parent += ","; + first = false; parent += indexFormatter(index); } - if(clique->parent()){ + if (clique->parent()) { parent += " : "; s << parentnum << "->" << num << "\n"; } first = true; - for(Key sep: clique->conditional_->parents()) { - if(!first) parent += ","; first = false; + for (Key sep : clique->conditional_->parents()) { + if (!first) parent += ","; + first = false; parent += indexFormatter(sep); } parent += "\"];\n"; s << parent; parentnum = num; - for(sharedClique c: clique->children) { + for (sharedClique c : clique->children) { num++; saveGraph(s, c, indexFormatter, parentnum); } From 4faac98f512314da1d5e0033b3897d43e2d5addd Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 21:00:49 -0400 Subject: [PATCH 02/13] Pointed pipeline to new docker image, using latest system Eigen3 --- bitbucket-pipelines.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bitbucket-pipelines.yml b/bitbucket-pipelines.yml index 2d8b7f061..288a40b14 100644 --- a/bitbucket-pipelines.yml +++ b/bitbucket-pipelines.yml @@ -3,14 +3,16 @@ # Only use spaces to indent your .yml configuration. # ----- # You can specify a custom docker image from Docker Hub as your build environment. -image: ilssim/cmake-boost-qt5 +image: dellaert/ubuntu-boost-tbb-eigen3:latest pipelines: default: - step: + caches: + - docker script: # Modify the commands below to build your repository. - mkdir build - cd build - - cmake .. + - cmake -DGTSAM_USE_SYSTEM_EIGEN=ON -DGTSAM_USE_EIGEN_MKL=OFF .. - make - make check \ No newline at end of file From e742cde2c8a53ba905f0f9ba44ae447a5d02d2a2 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 21:03:45 -0400 Subject: [PATCH 03/13] Fixed pipeline config --- bitbucket-pipelines.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/bitbucket-pipelines.yml b/bitbucket-pipelines.yml index 288a40b14..33204aea2 100644 --- a/bitbucket-pipelines.yml +++ b/bitbucket-pipelines.yml @@ -1,15 +1,12 @@ -# This is a sample build configuration for C++ – Make. -# Check our guides at https://confluence.atlassian.com/x/5Q4SMw for more examples. -# Only use spaces to indent your .yml configuration. +# Built from sample configuration for C++ – Make. +# Check https://confluence.atlassian.com/x/5Q4SMw for more examples. # ----- -# You can specify a custom docker image from Docker Hub as your build environment. +# Our custom docker image from Docker Hub as the build environment. image: dellaert/ubuntu-boost-tbb-eigen3:latest pipelines: default: - step: - caches: - - docker script: # Modify the commands below to build your repository. - mkdir build - cd build From 5d49177c8a50f8b0d394f93dfee4400150d45fe3 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 21:38:11 -0400 Subject: [PATCH 04/13] Added docker file to build pipeline docker image --- docker/ubuntu-boost-tbb-eigen3/Dockerfile | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 docker/ubuntu-boost-tbb-eigen3/Dockerfile diff --git a/docker/ubuntu-boost-tbb-eigen3/Dockerfile b/docker/ubuntu-boost-tbb-eigen3/Dockerfile new file mode 100644 index 000000000..b400c0538 --- /dev/null +++ b/docker/ubuntu-boost-tbb-eigen3/Dockerfile @@ -0,0 +1,18 @@ +# Get the base Ubuntu image from Docker Hub +FROM ubuntu:latest + +# Update apps on the base image +RUN apt-get -y update && apt-get install -y + +# Install C++ +RUN apt-get -y install build-essential + +# Install boost and cmake +RUN apt-get -y install libboost-all-dev cmake + +# Install TBB +RUN apt-get -y install libtbb-dev + +# Install latest Eigen +RUN apt-get install -y libeigen3-dev + From eb4659bcb66d7f7a55dd21e7f66cf10c449f0b62 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 21:38:32 -0400 Subject: [PATCH 05/13] Added -j4 flags --- bitbucket-pipelines.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bitbucket-pipelines.yml b/bitbucket-pipelines.yml index 33204aea2..8a8d5dd14 100644 --- a/bitbucket-pipelines.yml +++ b/bitbucket-pipelines.yml @@ -11,5 +11,5 @@ pipelines: - mkdir build - cd build - cmake -DGTSAM_USE_SYSTEM_EIGEN=ON -DGTSAM_USE_EIGEN_MKL=OFF .. - - make - - make check \ No newline at end of file + - make -j4 + - make -j4 check \ No newline at end of file From ca80678ffcdb069e0301e8c49a1efaf8ebabc9a3 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 21:38:50 -0400 Subject: [PATCH 06/13] Fixed more warnings --- examples/SolverComparer.cpp | 2 +- gtsam/3rdparty/metis/GKlib/csr.c | 31 +++++++++++++++---------------- gtsam/base/cholesky.cpp | 5 +++-- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/examples/SolverComparer.cpp b/examples/SolverComparer.cpp index deaf3b781..63c512edb 100644 --- a/examples/SolverComparer.cpp +++ b/examples/SolverComparer.cpp @@ -206,7 +206,7 @@ int main(int argc, char *argv[]) { } #ifdef GTSAM_USE_TBB - std::auto_ptr init; + std::unique_ptr init; if(nThreads > 0) { cout << "Using " << nThreads << " threads" << endl; init.reset(new tbb::task_scheduler_init(nThreads)); diff --git a/gtsam/3rdparty/metis/GKlib/csr.c b/gtsam/3rdparty/metis/GKlib/csr.c index 57934ee9d..4f6213c49 100644 --- a/gtsam/3rdparty/metis/GKlib/csr.c +++ b/gtsam/3rdparty/metis/GKlib/csr.c @@ -1349,27 +1349,26 @@ void gk_csr_Normalize(gk_csr_t *mat, int what, int norm) } } - if (what&GK_CSR_COL && mat->colval) { - n = mat->ncols; + if (what & GK_CSR_COL && mat->colval) { + n = mat->ncols; ptr = mat->colptr; val = mat->colval; - #pragma omp parallel if (ptr[n] > OMPMINOPS) +#pragma omp parallel if (ptr[n] > OMPMINOPS) { - #pragma omp for private(j,sum) schedule(static) - for (i=0; i 0) { - if (norm == 2) - sum=1.0/sqrt(sum); - else if (norm == 1) - sum=1.0/sum; - for (j=ptr[i]; j= 0 && nFrontal <= size_t(n)); + assert(ABC.rows() >= topleft); + const size_t n = static_cast(ABC.rows() - topleft); + assert(nFrontal <= size_t(n)); // Create views on blocks auto A = ABC.block(topleft, topleft, nFrontal, nFrontal); From 90373d878666e3e5719077512e965456c4b62720 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 21:51:20 -0400 Subject: [PATCH 07/13] Solved overloading, cherry-picked from fix/isam2 branch --- tests/testNonlinearOptimizer.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/testNonlinearOptimizer.cpp b/tests/testNonlinearOptimizer.cpp index 9ddbbb1b2..3d30d6880 100644 --- a/tests/testNonlinearOptimizer.cpp +++ b/tests/testNonlinearOptimizer.cpp @@ -20,13 +20,12 @@ #include #include #include -#include -#include #include #include #include #include #include +#include #include #include @@ -394,19 +393,20 @@ class IterativeLM: public LevenbergMarquardtOptimizer { /// Solver specific parameters ConjugateGradientParameters cgParams_; + Values initial_; public: /// Constructor IterativeLM(const NonlinearFactorGraph& graph, const Values& initialValues, const ConjugateGradientParameters &p, const LevenbergMarquardtParams& params = LevenbergMarquardtParams::LegacyDefaults()) : - LevenbergMarquardtOptimizer(graph, initialValues, params), cgParams_(p) { + LevenbergMarquardtOptimizer(graph, initialValues, params), cgParams_(p), initial_(initialValues) { } /// Solve that uses conjugate gradient virtual VectorValues solve(const GaussianFactorGraph &gfg, - const Values& initial, const NonlinearOptimizerParams& params) const { - VectorValues zeros = initial.zeroVectors(); + const NonlinearOptimizerParams& params) const { + VectorValues zeros = initial_.zeroVectors(); return conjugateGradientDescent(gfg, zeros, cgParams_); } }; From f60e3332a4a3fb1a0519f1a832de23ad9fa1b5c8 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 21:51:42 -0400 Subject: [PATCH 08/13] Made explicit float to avoid ambiguous overload. --- gtsam/geometry/tests/testPoint2.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gtsam/geometry/tests/testPoint2.cpp b/gtsam/geometry/tests/testPoint2.cpp index e2a5bcdea..8b9e8a7e6 100644 --- a/gtsam/geometry/tests/testPoint2.cpp +++ b/gtsam/geometry/tests/testPoint2.cpp @@ -100,12 +100,12 @@ TEST( Point2, expmap) { /* ************************************************************************* */ TEST( Point2, arithmetic) { - EXPECT(assert_equal( Point2(-5,-6), -Point2(5,6) )); - EXPECT(assert_equal( Point2(5,6), Point2(4,5)+Point2(1,1))); - EXPECT(assert_equal( Point2(3,4), Point2(4,5)-Point2(1,1) )); - EXPECT(assert_equal( Point2(8,6), Point2(4,3)*2)); - EXPECT(assert_equal( Point2(4,6), 2*Point2(2,3))); - EXPECT(assert_equal( Point2(2,3), Point2(4,6)/2)); + EXPECT(assert_equal(Point2(-5, -6), -Point2(5, 6))); + EXPECT(assert_equal(Point2(5, 6), Point2(4, 5) + Point2(1, 1))); + EXPECT(assert_equal(Point2(3, 4), Point2(4, 5) - Point2(1, 1))); + EXPECT(assert_equal(Point2(8, 6), Point2(4, 3) * 2)); + EXPECT(assert_equal(Point2(4, 6), 2.0 * Point2(2, 3))); + EXPECT(assert_equal(Point2(2, 3), Point2(4, 6) / 2)); } /* ************************************************************************* */ From 1bc479fc3c992b8009579ee8a1eaf61bfabb375d Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 22:54:36 -0400 Subject: [PATCH 09/13] Fixed some lint errors --- gtsam/geometry/Unit3.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/gtsam/geometry/Unit3.cpp b/gtsam/geometry/Unit3.cpp index dacb5c3fd..00edc1ad4 100644 --- a/gtsam/geometry/Unit3.cpp +++ b/gtsam/geometry/Unit3.cpp @@ -21,7 +21,7 @@ #include #include -#include // for GTSAM_USE_TBB +#include // for GTSAM_USE_TBB #ifdef __clang__ # pragma clang diagnostic push @@ -36,13 +36,14 @@ #include #include #include +#include using namespace std; namespace gtsam { /* ************************************************************************* */ -Unit3 Unit3::FromPoint3(const Point3& point, OptionalJacobian<2,3> H) { +Unit3 Unit3::FromPoint3(const Point3& point, OptionalJacobian<2, 3> H) { // 3*3 Derivative of representation with respect to point is 3*3: Matrix3 D_p_point; Unit3 direction; @@ -54,7 +55,7 @@ Unit3 Unit3::FromPoint3(const Point3& point, OptionalJacobian<2,3> H) { /* ************************************************************************* */ Unit3 Unit3::Random(boost::mt19937 & rng) { - // TODO allow any engine without including all of boost :-( + // TODO(dellaert): allow any engine without including all of boost :-( boost::uniform_on_sphere randomDirection(3); // This variate_generator object is required for versions of boost somewhere // around 1.46, instead of drawing directly using boost::uniform_on_sphere(rng). @@ -161,7 +162,8 @@ Matrix3 Unit3::skew() const { } /* ************************************************************************* */ -double Unit3::dot(const Unit3& q, OptionalJacobian<1,2> H_p, OptionalJacobian<1,2> H_q) const { +double Unit3::dot(const Unit3& q, OptionalJacobian<1, 2> H_p, + OptionalJacobian<1, 2> H_q) const { // Get the unit vectors of each, and the derivative. Matrix32 H_pn_p; Point3 pn = point3(H_p ? &H_pn_p : nullptr); @@ -185,7 +187,7 @@ double Unit3::dot(const Unit3& q, OptionalJacobian<1,2> H_p, OptionalJacobian<1, } /* ************************************************************************* */ -Vector2 Unit3::error(const Unit3& q, OptionalJacobian<2,2> H_q) const { +Vector2 Unit3::error(const Unit3& q, OptionalJacobian<2, 2> H_q) const { // 2D error is equal to B'*q, as B is 3x2 matrix and q is 3x1 const Vector2 xi = basis().transpose() * q.p_; if (H_q) { @@ -195,7 +197,8 @@ Vector2 Unit3::error(const Unit3& q, OptionalJacobian<2,2> H_q) const { } /* ************************************************************************* */ -Vector2 Unit3::errorVector(const Unit3& q, OptionalJacobian<2, 2> H_p, OptionalJacobian<2, 2> H_q) const { +Vector2 Unit3::errorVector(const Unit3& q, OptionalJacobian<2, 2> H_p, + OptionalJacobian<2, 2> H_q) const { // Get the point3 of this, and the derivative. Matrix32 H_qn_q; const Point3 qn = q.point3(H_q ? &H_qn_q : nullptr); @@ -230,7 +233,7 @@ Vector2 Unit3::errorVector(const Unit3& q, OptionalJacobian<2, 2> H_p, OptionalJ } /* ************************************************************************* */ -double Unit3::distance(const Unit3& q, OptionalJacobian<1,2> H) const { +double Unit3::distance(const Unit3& q, OptionalJacobian<1, 2> H) const { Matrix2 H_xi_q; const Vector2 xi = error(q, H ? &H_xi_q : nullptr); const double theta = xi.norm(); @@ -277,4 +280,4 @@ Vector2 Unit3::localCoordinates(const Unit3& other) const { } /* ************************************************************************* */ -} +} // namespace gtsam From b9f080456c4164f11812c6ae1f960bd6adb741ca Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 22:54:48 -0400 Subject: [PATCH 10/13] Catch exception by value --- wrap/FileWriter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wrap/FileWriter.cpp b/wrap/FileWriter.cpp index 783661819..2c3843b37 100644 --- a/wrap/FileWriter.cpp +++ b/wrap/FileWriter.cpp @@ -29,7 +29,7 @@ void FileWriter::emit(bool add_header, bool force_overwrite) const { bool file_exists = true; try { existing_contents = file_contents(filename_.c_str(), add_header); - } catch (CantOpenFile) { + } catch (const CantOpenFile& e) { file_exists = false; } From 845574860fb7dbbf631de84067036e79ad15ec21 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 22:55:46 -0400 Subject: [PATCH 11/13] Try cosmic rather than latest (which throws internal compiler error) --- docker/ubuntu-boost-tbb-eigen3/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/ubuntu-boost-tbb-eigen3/Dockerfile b/docker/ubuntu-boost-tbb-eigen3/Dockerfile index b400c0538..bb2bf4719 100644 --- a/docker/ubuntu-boost-tbb-eigen3/Dockerfile +++ b/docker/ubuntu-boost-tbb-eigen3/Dockerfile @@ -1,5 +1,5 @@ # Get the base Ubuntu image from Docker Hub -FROM ubuntu:latest +FROM ubuntu:cosmic # Update apps on the base image RUN apt-get -y update && apt-get install -y From a932d4bd5e06f9b906a70a592a079669042198c0 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 23:13:15 -0400 Subject: [PATCH 12/13] Trying xenial --- bitbucket-pipelines.yml | 2 +- docker/ubuntu-boost-tbb-eigen3/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bitbucket-pipelines.yml b/bitbucket-pipelines.yml index 8a8d5dd14..58e27c79a 100644 --- a/bitbucket-pipelines.yml +++ b/bitbucket-pipelines.yml @@ -2,7 +2,7 @@ # Check https://confluence.atlassian.com/x/5Q4SMw for more examples. # ----- # Our custom docker image from Docker Hub as the build environment. -image: dellaert/ubuntu-boost-tbb-eigen3:latest +image: dellaert/ubuntu-boost-tbb-eigen3:xenial pipelines: default: diff --git a/docker/ubuntu-boost-tbb-eigen3/Dockerfile b/docker/ubuntu-boost-tbb-eigen3/Dockerfile index bb2bf4719..dda4fbbaf 100644 --- a/docker/ubuntu-boost-tbb-eigen3/Dockerfile +++ b/docker/ubuntu-boost-tbb-eigen3/Dockerfile @@ -1,5 +1,5 @@ # Get the base Ubuntu image from Docker Hub -FROM ubuntu:cosmic +FROM ubuntu:xenial # Update apps on the base image RUN apt-get -y update && apt-get install -y From 632dcd0bf2cf60e755bb40216f85706b6222f611 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Mon, 8 Oct 2018 23:24:19 -0400 Subject: [PATCH 13/13] Trying bionic, again, with -j2 flag --- bitbucket-pipelines.yml | 6 +++--- docker/ubuntu-boost-tbb-eigen3/Dockerfile | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bitbucket-pipelines.yml b/bitbucket-pipelines.yml index 58e27c79a..d41ba7f26 100644 --- a/bitbucket-pipelines.yml +++ b/bitbucket-pipelines.yml @@ -2,7 +2,7 @@ # Check https://confluence.atlassian.com/x/5Q4SMw for more examples. # ----- # Our custom docker image from Docker Hub as the build environment. -image: dellaert/ubuntu-boost-tbb-eigen3:xenial +image: dellaert/ubuntu-boost-tbb-eigen3:bionic pipelines: default: @@ -11,5 +11,5 @@ pipelines: - mkdir build - cd build - cmake -DGTSAM_USE_SYSTEM_EIGEN=ON -DGTSAM_USE_EIGEN_MKL=OFF .. - - make -j4 - - make -j4 check \ No newline at end of file + - make -j2 + - make -j2 check \ No newline at end of file diff --git a/docker/ubuntu-boost-tbb-eigen3/Dockerfile b/docker/ubuntu-boost-tbb-eigen3/Dockerfile index dda4fbbaf..33aa1ab96 100644 --- a/docker/ubuntu-boost-tbb-eigen3/Dockerfile +++ b/docker/ubuntu-boost-tbb-eigen3/Dockerfile @@ -1,5 +1,5 @@ # Get the base Ubuntu image from Docker Hub -FROM ubuntu:xenial +FROM ubuntu:bionic # Update apps on the base image RUN apt-get -y update && apt-get install -y