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.

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

GTSAM: Fix C++20 compatibility issues, unvendor Spectra #25597

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions recipes/gtsam/all/conan_deps.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Replace vendored Spectra with a Conan version
find_package(spectra REQUIRED CONFIG)
link_libraries(Spectra::Spectra)
8 changes: 8 additions & 0 deletions recipes/gtsam/all/conandata.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ patches:
- patch_file: "patches/4.2.0-0002-eigen3-version.patch"
patch_description: "Get Eigen3 version info from Conan"
patch_type: "conan"
- patch_file: "patches/4.2.0-0003-update-spectra.patch"
patch_description: "Update Spectra to v1.0 for C++20 support"
patch_type: "portability"
patch_source: "https://github.com/borglab/gtsam/pull/1871"
"4.1.1":
- patch_file: "patches/4.1.1-0001-cmake-boost.patch"
patch_description: "Use boost targets"
Expand All @@ -35,6 +39,10 @@ patches:
- patch_file: "patches/4.1.1-0005-eigen3-version.patch"
patch_description: "Get Eigen3 version info from Conan"
patch_type: "conan"
- patch_file: "patches/4.1.1-0006-update-spectra.patch"
patch_description: "Update Spectra to v1.0 for C++20 support"
patch_type: "portability"
patch_source: "https://github.com/borglab/gtsam/pull/1871"
"4.0.3":
- patch_file: "patches/4.0.3-0001-cmake-boost.patch"
patch_description: "Use boost targets"
Expand Down
15 changes: 15 additions & 0 deletions recipes/gtsam/all/conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class GtsamConan(ConanFile):

def export_sources(self):
export_conandata_patches(self)
copy(self, "conan_deps.cmake", self.recipe_folder, os.path.join(self.export_sources_folder, "src"))

def config_options(self):
if self.settings.os == "Windows":
Expand All @@ -136,6 +137,8 @@ def layout(self):
def requirements(self):
self.requires("boost/1.84.0", transitive_headers=True)
self.requires("eigen/3.4.0", transitive_headers=True)
if Version(self.version) >= "4.1":
self.requires("spectra/1.0.1")
if self.options.with_TBB:
if Version(self.version) >= "4.1":
self.requires("onetbb/2021.10.0", transitive_headers=True, transitive_libs=True)
Expand Down Expand Up @@ -201,6 +204,8 @@ def source(self):

def generate(self):
tc = CMakeToolchain(self)
if Version(self.version) >= "4.1":
tc.variables["CMAKE_PROJECT_GTSAM_INCLUDE"] = "conan_deps.cmake"
# https://github.com/borglab/gtsam/blob/4.2.0/cmake/HandleGeneralOptions.cmake
tc.variables["GTSAM_BUILD_UNSTABLE"] = self.options.build_unstable
tc.variables["GTSAM_UNSTABLE_BUILD_PYTHON"] = False
Expand Down Expand Up @@ -316,6 +321,14 @@ def _patch_sources(self):
"list(APPEND GTSAM_ADDITIONAL_LIBRARIES tbb tbbmalloc)",
"list(APPEND GTSAM_ADDITIONAL_LIBRARIES TBB::tbb TBB::tbbmalloc)")

# Unvendor Spectra
rmdir(self, os.path.join(self.source_folder, "gtsam", "3rdparty", "Spectra"))

# Remove an unused header from the list of precompiled headers,
# which fails with a compilation error for C++20 on MSVC.
# https://github.com/borglab/gtsam/pull/1870
replace_in_file(self, os.path.join(self.source_folder, "gtsam", "precompiled_header.h"),
"#include <gtsam/base/chartTesting.h>", "")

def build(self):
self._patch_sources()
Expand Down Expand Up @@ -367,6 +380,8 @@ def package_info(self):
gtsam.libs = ["gtsam"]
gtsam.requires = [f"boost::{component}" for component in self._required_boost_components]
gtsam.requires.append("eigen::eigen")
if Version(self.version) >= "4.1":
gtsam.requires.append("spectra::spectra")
if self.options.with_TBB:
gtsam.requires.append("onetbb::onetbb")
if self.options.default_allocator == "tcmalloc":
Expand Down
65 changes: 65 additions & 0 deletions recipes/gtsam/all/patches/4.1.1-0006-update-spectra.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
diff --git a/gtsam/sfm/ShonanAveraging.cpp b/gtsam/sfm/ShonanAveraging.cpp
--- a/gtsam/sfm/ShonanAveraging.cpp
+++ b/gtsam/sfm/ShonanAveraging.cpp
@@ -16,7 +16,7 @@
* @brief Shonan Averaging algorithm
*/

-#include <SymEigsSolver.h>
+#include <Spectra/SymEigsSolver.h>
#include <cmath>
#include <gtsam/linear/PCGSolver.h>
#include <gtsam/linear/SubgraphPreconditioner.h>
@@ -576,6 +576,8 @@
* nontrivial function, perform_op(x,y), that computes and returns the product
* y = (A + sigma*I) x */
struct MatrixProdFunctor {
+ using Scalar = double;
+
// Const reference to an externally-held matrix whose minimum-eigenvalue we
// want to compute
const Sparse &A_;
@@ -636,13 +638,13 @@
Eigen::Index numLanczosVectors = 20) {
// a. Estimate the largest-magnitude eigenvalue of this matrix using Lanczos
MatrixProdFunctor lmOperator(A);
- Spectra::SymEigsSolver<double, Spectra::SELECT_EIGENVALUE::LARGEST_MAGN,
- MatrixProdFunctor>
- lmEigenValueSolver(&lmOperator, 1, std::min(numLanczosVectors, A.rows()));
+ Spectra::SymEigsSolver lmEigenValueSolver(
+ lmOperator, 1, std::min(numLanczosVectors, A.rows()));
lmEigenValueSolver.init();

- const int lmConverged = lmEigenValueSolver.compute(
- maxIterations, 1e-4, Spectra::SELECT_EIGENVALUE::LARGEST_MAGN);
+ const int lmConverged =
+ lmEigenValueSolver.compute(Spectra::SortRule::LargestMagn, maxIterations,
+ 1e-4, Spectra::SortRule::LargestMagn);

// Check convergence and bail out if necessary
if (lmConverged != 1) return false;
@@ -670,10 +672,8 @@

MatrixProdFunctor minShiftedOperator(A, -2 * lmEigenValue);

- Spectra::SymEigsSolver<double, Spectra::SELECT_EIGENVALUE::LARGEST_MAGN,
- MatrixProdFunctor>
- minEigenValueSolver(&minShiftedOperator, 1,
- std::min(numLanczosVectors, A.rows()));
+ Spectra::SymEigsSolver minEigenValueSolver(
+ minShiftedOperator, 1, std::min(numLanczosVectors, A.rows()));

// If S is a critical point of F, then S^T is also in the null space of S -
// Lambda(S) (cf. Lemma 6 of the tech report), and therefore its rows are
@@ -701,8 +701,9 @@
// order to be able to estimate the smallest eigenvalue within an *absolute*
// tolerance of 'minEigenvalueNonnegativityTolerance'
const int minConverged = minEigenValueSolver.compute(
- maxIterations, minEigenvalueNonnegativityTolerance / lmEigenValue,
- Spectra::SELECT_EIGENVALUE::LARGEST_MAGN);
+ Spectra::SortRule::LargestMagn, maxIterations,
+ minEigenvalueNonnegativityTolerance / lmEigenValue,
+ Spectra::SortRule::LargestMagn);

if (minConverged != 1) return false;

65 changes: 65 additions & 0 deletions recipes/gtsam/all/patches/4.2.0-0003-update-spectra.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
diff --git a/gtsam/sfm/ShonanAveraging.cpp b/gtsam/sfm/ShonanAveraging.cpp
--- a/gtsam/sfm/ShonanAveraging.cpp
+++ b/gtsam/sfm/ShonanAveraging.cpp
@@ -16,7 +16,7 @@
* @brief Shonan Averaging algorithm
*/

-#include <SymEigsSolver.h>
+#include <Spectra/SymEigsSolver.h>
#include <cmath>
#include <gtsam/linear/PCGSolver.h>
#include <gtsam/linear/SubgraphPreconditioner.h>
@@ -576,6 +576,8 @@
* nontrivial function, perform_op(x,y), that computes and returns the product
* y = (A + sigma*I) x */
struct MatrixProdFunctor {
+ using Scalar = double;
+
// Const reference to an externally-held matrix whose minimum-eigenvalue we
// want to compute
const Sparse &A_;
@@ -636,13 +638,13 @@
Eigen::Index numLanczosVectors = 20) {
// a. Estimate the largest-magnitude eigenvalue of this matrix using Lanczos
MatrixProdFunctor lmOperator(A);
- Spectra::SymEigsSolver<double, Spectra::SELECT_EIGENVALUE::LARGEST_MAGN,
- MatrixProdFunctor>
- lmEigenValueSolver(&lmOperator, 1, std::min(numLanczosVectors, A.rows()));
+ Spectra::SymEigsSolver lmEigenValueSolver(
+ lmOperator, 1, std::min(numLanczosVectors, A.rows()));
lmEigenValueSolver.init();

- const int lmConverged = lmEigenValueSolver.compute(
- maxIterations, 1e-4, Spectra::SELECT_EIGENVALUE::LARGEST_MAGN);
+ const int lmConverged =
+ lmEigenValueSolver.compute(Spectra::SortRule::LargestMagn, maxIterations,
+ 1e-4, Spectra::SortRule::LargestMagn);

// Check convergence and bail out if necessary
if (lmConverged != 1) return false;
@@ -670,10 +672,8 @@

MatrixProdFunctor minShiftedOperator(A, -2 * lmEigenValue);

- Spectra::SymEigsSolver<double, Spectra::SELECT_EIGENVALUE::LARGEST_MAGN,
- MatrixProdFunctor>
- minEigenValueSolver(&minShiftedOperator, 1,
- std::min(numLanczosVectors, A.rows()));
+ Spectra::SymEigsSolver minEigenValueSolver(
+ minShiftedOperator, 1, std::min(numLanczosVectors, A.rows()));

// If S is a critical point of F, then S^T is also in the null space of S -
// Lambda(S) (cf. Lemma 6 of the tech report), and therefore its rows are
@@ -701,8 +701,9 @@
// order to be able to estimate the smallest eigenvalue within an *absolute*
// tolerance of 'minEigenvalueNonnegativityTolerance'
const int minConverged = minEigenValueSolver.compute(
- maxIterations, minEigenvalueNonnegativityTolerance / lmEigenValue,
- Spectra::SELECT_EIGENVALUE::LARGEST_MAGN);
+ Spectra::SortRule::LargestMagn, maxIterations,
+ minEigenvalueNonnegativityTolerance / lmEigenValue,
+ Spectra::SortRule::LargestMagn);

if (minConverged != 1) return false;

Loading