From 11e6376675fbe8ff7b0dfeb3f36fa416a64f10e3 Mon Sep 17 00:00:00 2001
From: Alex Stewart <alexs.mac@gmail.com>
Date: Wed, 22 Jun 2022 19:05:30 +0100
Subject: [PATCH] Fix #ifdef guards around METIS usage in EigenSparse backend
 and tests

Change-Id: Idd1e0b7b1b2df2d402431b107dee0fa8e0fd58f5
---
 internal/ceres/dynamic_sparsity_test.cc | 10 +++++++++-
 internal/ceres/eigensparse.cc           | 18 +++++-------------
 internal/ceres/solver.cc                |  4 ++++
 internal/ceres/sparse_cholesky_test.cc  |  4 ++++
 4 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/internal/ceres/dynamic_sparsity_test.cc b/internal/ceres/dynamic_sparsity_test.cc
index 694a343df..917309858 100644
--- a/internal/ceres/dynamic_sparsity_test.cc
+++ b/internal/ceres/dynamic_sparsity_test.cc
@@ -366,7 +366,8 @@ class EuclideanDistanceFunctor {
 };
 
 TEST(DynamicSparsity, StaticAndDynamicSparsityProduceSameSolution) {
-  // Skip test if there is no sparse linear algebra library.
+  // Skip test if there is no sparse linear algebra library that
+  // supports dynamic sparsity.
   if (!IsSparseLinearAlgebraLibraryTypeAvailable(SUITE_SPARSE) &&
       !IsSparseLinearAlgebraLibraryTypeAvailable(EIGEN_SPARSE)) {
     return;
@@ -426,6 +427,13 @@ TEST(DynamicSparsity, StaticAndDynamicSparsityProduceSameSolution) {
   Solver::Options options;
   options.max_num_iterations = 100;
   options.linear_solver_type = SPARSE_NORMAL_CHOLESKY;
+  // Only SuiteSparse & EigenSparse currently support dynamic sparsity.
+  options.sparse_linear_algebra_library_type =
+#if !defined(CERES_NO_SUITESPARSE)
+      ceres::SUITE_SPARSE;
+#elif defined(CERES_USE_EIGEN_SPARSE)
+      ceres::EIGEN_SPARSE;
+#endif
 
   // First, solve `X` and `t` jointly with dynamic_sparsity = true.
   Matrix X0 = X;
diff --git a/internal/ceres/eigensparse.cc b/internal/ceres/eigensparse.cc
index 982608a1d..a2407c0fa 100644
--- a/internal/ceres/eigensparse.cc
+++ b/internal/ceres/eigensparse.cc
@@ -157,23 +157,19 @@ std::unique_ptr<SparseCholesky> EigenSparseCholesky::Create(
 
   if (ordering_type == OrderingType::AMD) {
     return std::make_unique<EigenSparseCholeskyTemplate<WithAMDOrdering>>();
-#ifndef CERES_NO_EIGEN_METIS
   } else if (ordering_type == OrderingType::NESDIS) {
+#ifndef CERES_NO_EIGEN_METIS
     using WithMetisOrdering = Eigen::SimplicialLDLT<Eigen::SparseMatrix<double>,
                                                     Eigen::Upper,
                                                     Eigen::MetisOrdering<int>>;
-
     return std::make_unique<EigenSparseCholeskyTemplate<WithMetisOrdering>>();
-  }
 #else
-  } else {
     LOG(FATAL)
         << "Congratulations you have found a bug in Ceres Solver. Please "
            "report it to the Ceres Solver developers.";
     return nullptr;
+#endif  // CERES_NO_EIGEN_METIS
   }
-#endif
-
   return std::make_unique<EigenSparseCholeskyTemplate<WithNaturalOrdering>>();
 }
 
@@ -190,23 +186,19 @@ std::unique_ptr<SparseCholesky> FloatEigenSparseCholesky::Create(
                             Eigen::NaturalOrdering<int>>;
   if (ordering_type == OrderingType::AMD) {
     return std::make_unique<EigenSparseCholeskyTemplate<WithAMDOrdering>>();
-#ifndef CERES_NO_EIGEN_METIS
   } else if (ordering_type == OrderingType::NESDIS) {
+#ifndef CERES_NO_EIGEN_METIS
     using WithMetisOrdering = Eigen::SimplicialLDLT<Eigen::SparseMatrix<float>,
                                                     Eigen::Upper,
                                                     Eigen::MetisOrdering<int>>;
-
     return std::make_unique<EigenSparseCholeskyTemplate<WithMetisOrdering>>();
-  }
 #else
-  } else {
     LOG(FATAL)
         << "Congratulations you have found a bug in Ceres Solver. Please "
-           "report it to the Ceres Solver developers.";
+        "report it to the Ceres Solver developers.";
     return nullptr;
+#endif  // CERES_NO_EIGEN_METIS
   }
-#endif
-
   return std::make_unique<EigenSparseCholeskyTemplate<WithNaturalOrdering>>();
 }
 
diff --git a/internal/ceres/solver.cc b/internal/ceres/solver.cc
index 47a370d97..950384340 100644
--- a/internal/ceres/solver.cc
+++ b/internal/ceres/solver.cc
@@ -213,6 +213,10 @@ bool TrustRegionOptionsAreValid(const Solver::Options& options, string* error) {
           *error = StringPrintf(
               "Can't use NESDIS with SUITE_SPARSE because SuiteSparse was "
               "compiled without support for Metis.");
+        } else if (options.sparse_linear_algebra_library_type == EIGEN_SPARSE) {
+          *error = StringPrintf(
+              "Can't use NESDIS with EIGEN_SPARSE because Ceres was "
+              "compiled without support for Metis.");
         } else {
           *error = StringPrintf(
               "Can't use NESDIS with "
diff --git a/internal/ceres/sparse_cholesky_test.cc b/internal/ceres/sparse_cholesky_test.cc
index 785f2e912..147711ed6 100644
--- a/internal/ceres/sparse_cholesky_test.cc
+++ b/internal/ceres/sparse_cholesky_test.cc
@@ -230,7 +230,9 @@ INSTANTIATE_TEST_SUITE_P(
     SparseCholeskyTest,
     ::testing::Combine(::testing::Values(EIGEN_SPARSE),
                        ::testing::Values(OrderingType::AMD,
+#ifndef CERES_NO_EIGEN_METIS
                                          OrderingType::NESDIS,
+#endif  // CERES_NO_EIGEN_METIS
                                          OrderingType::NATURAL),
                        ::testing::Values(true, false)),
     ParamInfoToString);
@@ -240,7 +242,9 @@ INSTANTIATE_TEST_SUITE_P(
     SparseCholeskyTest,
     ::testing::Combine(::testing::Values(EIGEN_SPARSE),
                        ::testing::Values(OrderingType::AMD,
+#ifndef CERES_NO_EIGEN_METIS
                                          OrderingType::NESDIS,
+#endif  // CERES_NO_EIGEN_METIS
                                          OrderingType::NATURAL),
                        ::testing::Values(true, false)),
     ParamInfoToString);