Merge Factory improvements

* Make all iterative solver parameter classes based on the same structure * Allow passing factory parameter instances instead of full factories for criteria, preconditioners etc. * Remove now unnecessary `.on(...)` calls. * More strict const-correctness requirements for `with_*` functions Related PR: #1336
ginkgo-project · Oct 9, 2023 · 8368485 · 8368485
2 parents 4f3b8c6 + af0b8c7
commit 8368485
Show file tree

Hide file tree

Showing 109 changed files with 1,859 additions and 1,559 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -511,3 +511,8 @@ else()
     FILE(READ ${PROJECT_BINARY_DIR}/minimal.log GINKGO_LOG_SUMMARY)
 endif()
 MESSAGE(STATUS "${GINKGO_LOG_SUMMARY}")
+
+# make sure no build files get committed accidentally
+if(NOT EXISTS ${CMAKE_CURRENT_BINARY_DIR}/.gitignore)
+    file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/.gitignore "*")
+endif()
diff --git a/benchmark/solver/solver_common.hpp b/benchmark/solver/solver_common.hpp
@@ -239,21 +239,18 @@ std::unique_ptr<gko::LinOpFactory> generate_solver(
         return gko::experimental::solver::Direct<etype, itype>::build()
             .with_factorization(
                 gko::experimental::factorization::Cholesky<etype,
-                                                           itype>::build()
-                    .on(exec))
+                                                           itype>::build())
             .on(exec);
     } else if (description == "symm_direct") {
         return gko::experimental::solver::Direct<etype, itype>::build()
             .with_factorization(
                 gko::experimental::factorization::Lu<etype, itype>::build()
-                    .with_symmetric_sparsity(true)
-                    .on(exec))
+                    .with_symmetric_sparsity(true))
             .on(exec);
     } else if (description == "direct") {
         return gko::experimental::solver::Direct<etype, itype>::build()
             .with_factorization(
-                gko::experimental::factorization::Lu<etype, itype>::build().on(
-                    exec))
+                gko::experimental::factorization::Lu<etype, itype>::build())
             .on(exec);
     } else if (description == "overhead") {
         return add_criteria_precond_finalize<gko::Overhead<etype>>(

diff --git a/benchmark/utils/overhead_linop.hpp b/benchmark/utils/overhead_linop.hpp
@@ -104,27 +104,12 @@ class Overhead : public EnableLinOp<Overhead<ValueType>>,
     friend class EnablePolymorphicObject<Overhead, LinOp>;
 
 public:
-    GKO_CREATE_FACTORY_PARAMETERS(parameters, Factory)
-    {
-        /**
-         * Criterion factories.
-         */
-        std::vector<std::shared_ptr<const stop::CriterionFactory>>
-            GKO_FACTORY_PARAMETER_VECTOR(criteria, nullptr);
-
-        /**
-         * Preconditioner factory.
-         */
-        std::shared_ptr<const LinOpFactory> GKO_FACTORY_PARAMETER_SCALAR(
-            preconditioner, nullptr);
-
-        /**
-         * Already generated preconditioner. If one is provided, the factory
-         * `preconditioner` will be ignored.
-         */
-        std::shared_ptr<const LinOp> GKO_FACTORY_PARAMETER_SCALAR(
-            generated_preconditioner, nullptr);
-    };
+    class Factory;
+
+    struct parameters_type
+        : public gko::solver::
+              enable_preconditioned_iterative_solver_factory_parameters<
+                  parameters_type, Factory> {};
 
     GKO_ENABLE_LIN_OP_FACTORY(Overhead, parameters, Factory);
     GKO_ENABLE_BUILD_METHOD(Factory);

diff --git a/benchmark/utils/preconditioners.hpp b/benchmark/utils/preconditioners.hpp
@@ -122,7 +122,7 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
                                 .on(exec));
              return gko::preconditioner::Ic<gko::solver::LowerTrs<etype, itype>,
                                             itype>::build()
-                 .with_factorization_factory(fact)
+                 .with_factorization(fact)
                  .on(exec);
          }},
         {"parict",
@@ -137,7 +137,7 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
              return gko::preconditioner::
                  Ilu<gko::solver::LowerTrs<etype, itype>,
                      gko::solver::UpperTrs<etype, itype>, false, itype>::build()
-                     .with_factorization_factory(fact)
+                     .with_factorization(fact)
                      .on(exec);
          }},
         {"parilu",
@@ -150,7 +150,7 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
              return gko::preconditioner::
                  Ilu<gko::solver::LowerTrs<etype, itype>,
                      gko::solver::UpperTrs<etype, itype>, false, itype>::build()
-                     .with_factorization_factory(fact)
+                     .with_factorization(fact)
                      .on(exec);
          }},
         {"parilut",
@@ -165,7 +165,7 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
              return gko::preconditioner::
                  Ilu<gko::solver::LowerTrs<etype, itype>,
                      gko::solver::UpperTrs<etype, itype>, false, itype>::build()
-                     .with_factorization_factory(fact)
+                     .with_factorization(fact)
                      .on(exec);
          }},
         {"ic",
@@ -174,7 +174,7 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
                  gko::factorization::Ic<etype, itype>::build().on(exec));
              return gko::preconditioner::Ic<gko::solver::LowerTrs<etype, itype>,
                                             itype>::build()
-                 .with_factorization_factory(fact)
+                 .with_factorization(fact)
                  .on(exec);
          }},
         {"ilu",
@@ -184,7 +184,7 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
              return gko::preconditioner::
                  Ilu<gko::solver::LowerTrs<etype, itype>,
                      gko::solver::UpperTrs<etype, itype>, false, itype>::build()
-                     .with_factorization_factory(fact)
+                     .with_factorization(fact)
                      .on(exec);
          }},
         {"paric-isai",
@@ -201,8 +201,8 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
              return gko::preconditioner::Ic<
                         gko::preconditioner::LowerIsai<etype, itype>,
                         itype>::build()
-                 .with_factorization_factory(fact)
-                 .with_l_solver_factory(lisai)
+                 .with_factorization(fact)
+                 .with_l_solver(lisai)
                  .on(exec);
          }},
         {"parict-isai",
@@ -221,8 +221,8 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
              return gko::preconditioner::Ic<
                         gko::preconditioner::LowerIsai<etype, itype>,
                         itype>::build()
-                 .with_factorization_factory(fact)
-                 .with_l_solver_factory(lisai)
+                 .with_factorization(fact)
+                 .with_l_solver(lisai)
                  .on(exec);
          }},
         {"parilu-isai",
@@ -244,9 +244,9 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
                         gko::preconditioner::LowerIsai<etype, itype>,
                         gko::preconditioner::UpperIsai<etype, itype>, false,
                         itype>::build()
-                 .with_factorization_factory(fact)
-                 .with_l_solver_factory(lisai)
-                 .with_u_solver_factory(uisai)
+                 .with_factorization(fact)
+                 .with_l_solver(lisai)
+                 .with_u_solver(uisai)
                  .on(exec);
          }},
         {"parilut-isai",
@@ -270,9 +270,9 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
                         gko::preconditioner::LowerIsai<etype, itype>,
                         gko::preconditioner::UpperIsai<etype, itype>, false,
                         itype>::build()
-                 .with_factorization_factory(fact)
-                 .with_l_solver_factory(lisai)
-                 .with_u_solver_factory(uisai)
+                 .with_factorization(fact)
+                 .with_l_solver(lisai)
+                 .with_u_solver(uisai)
                  .on(exec);
          }},
         {"ic-isai",
@@ -286,8 +286,8 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
              return gko::preconditioner::Ic<
                         gko::preconditioner::LowerIsai<etype, itype>,
                         itype>::build()
-                 .with_factorization_factory(fact)
-                 .with_l_solver_factory(lisai)
+                 .with_factorization(fact)
+                 .with_l_solver(lisai)
                  .on(exec);
          }},
         {"ilu-isai",
@@ -306,9 +306,9 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
                         gko::preconditioner::LowerIsai<etype, itype>,
                         gko::preconditioner::UpperIsai<etype, itype>, false,
                         itype>::build()
-                 .with_factorization_factory(fact)
-                 .with_l_solver_factory(lisai)
-                 .with_u_solver_factory(uisai)
+                 .with_factorization(fact)
+                 .with_l_solver(lisai)
+                 .with_u_solver(uisai)
                  .on(exec);
          }},
         {"general-isai",
@@ -326,8 +326,7 @@ const std::map<std::string, std::function<std::unique_ptr<gko::LinOpFactory>(
         {"overhead", [](std::shared_ptr<const gko::Executor> exec) {
              return gko::Overhead<etype>::build()
                  .with_criteria(gko::stop::ResidualNorm<etype>::build()
-                                    .with_reduction_factor(rc_etype{})
-                                    .on(exec))
+                                    .with_reduction_factor(rc_etype{}))
                  .on(exec);
          }}};
 

diff --git a/core/distributed/preconditioner/schwarz.cpp b/core/distributed/preconditioner/schwarz.cpp
@@ -102,8 +102,8 @@ template <typename ValueType, typename LocalIndexType, typename GlobalIndexType>
 void Schwarz<ValueType, LocalIndexType, GlobalIndexType>::generate(
     std::shared_ptr<const LinOp> system_matrix)
 {
-    if (parameters_.local_solver_factory) {
-        this->local_solver_ = parameters_.local_solver_factory->generate(
+    if (parameters_.local_solver) {
+        this->local_solver_ = parameters_.local_solver->generate(
             as<experimental::distributed::Matrix<ValueType, LocalIndexType,
                                                  GlobalIndexType>>(
                 system_matrix)

diff --git a/core/preconditioner/isai.cpp b/core/preconditioner/isai.cpp
@@ -230,17 +230,15 @@ void Isai<IsaiType, ValueType, IndexType>::generate_inverse(
                 excess_solver_factory =
                     Gmres::build()
                         .with_preconditioner(
-                            Bj::build().with_max_block_size(32u).on(exec))
+                            Bj::build().with_max_block_size(32u))
                         .with_criteria(
-                            gko::stop::Iteration::build()
-                                .with_max_iters(excess_dim)
-                                .on(exec),
+                            gko::stop::Iteration::build().with_max_iters(
+                                excess_dim),
                             gko::stop::ResidualNorm<ValueType>::build()
                                 .with_baseline(gko::stop::mode::rhs_norm)
                                 .with_reduction_factor(
                                     remove_complex<ValueType>{
-                                        excess_solver_reduction})
-                                .on(exec))
+                                        excess_solver_reduction}))
                         .on(exec);
                 excess_solution->copy_from(excess_rhs);
             } else if (is_lower) {

diff --git a/core/solver/multigrid.cpp b/core/solver/multigrid.cpp
@@ -569,30 +569,26 @@ void Multigrid::generate()
                     using absolute_value_type = remove_complex<value_type>;
                     return solver::Gmres<value_type>::build()
                         .with_criteria(
-                            stop::Iteration::build()
-                                .with_max_iters(matrix->get_size()[0])
-                                .on(exec),
+                            stop::Iteration::build().with_max_iters(
+                                matrix->get_size()[0]),
                             stop::ResidualNorm<value_type>::build()
                                 .with_reduction_factor(
                                     std::numeric_limits<
                                         absolute_value_type>::epsilon() *
-                                    absolute_value_type{10})
-                                .on(exec))
+                                    absolute_value_type{10}))
                         .with_krylov_dim(
                             std::min(size_type(100), matrix->get_size()[0]))
                         .with_preconditioner(
                             preconditioner::Jacobi<value_type>::build()
-                                .with_max_block_size(1u)
-                                .on(exec))
+                                .with_max_block_size(1u))
                         .on(exec)
                         ->generate(matrix);
                 } else {
                     return experimental::solver::Direct<value_type,
                                                         int32>::build()
                         .with_factorization(
                             experimental::factorization::Lu<value_type,
-                                                            int32>::build()
-                                .on(exec))
+                                                            int32>::build())
                         .on(exec)
                         ->generate(matrix);
                 }

diff --git a/core/test/log/convergence.cpp b/core/test/log/convergence.cpp
@@ -68,8 +68,7 @@ class Convergence : public ::testing::Test {
         gko::initialize<AbsoluteDense>({6}, exec);
     std::unique_ptr<gko::LinOp> system =
         gko::solver::Ir<T>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(1u).on(exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(1u))
             .on(exec)
             ->generate(gko::initialize<Dense>(I<I<T>>{{1, 2}, {0, 3}}, exec));
     std::unique_ptr<Dense> rhs = gko::initialize<Dense>({15, 25}, exec);

diff --git a/core/test/log/papi.cpp b/core/test/log/papi.cpp
@@ -471,8 +471,7 @@ TYPED_TEST(Papi, CatchesLinOpFactoryGenerateStarted)
 {
     auto factory =
         gko::solver::Bicgstab<TypeParam>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(3u).on(this->exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(3u))
             .on(this->exec);
     auto str = this->init(gko::log::Logger::linop_factory_generate_started_mask,
                           "linop_factory_generate_started", factory.get());
@@ -492,8 +491,7 @@ TYPED_TEST(Papi, CatchesLinOpFactoryGenerateCompleted)
 {
     auto factory =
         gko::solver::Bicgstab<TypeParam>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(3u).on(this->exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(3u))
             .on(this->exec);
     TypeParam dummy;
     auto str =

diff --git a/core/test/log/profiler_hook.cpp b/core/test/log/profiler_hook.cpp
@@ -202,8 +202,7 @@ TEST(ProfilerHook, LogsIteration)
     auto alpha = gko::share(gko::initialize<Vec>({1.0}, exec));
     auto solver =
         gko::solver::Ir<>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(1u).on(exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(1u))
             .on(exec)
             ->generate(mtx);
     logger->set_object_name(solver, "solver");

diff --git a/core/test/log/record.cpp b/core/test/log/record.cpp
@@ -440,8 +440,7 @@ TEST(Record, CatchesLinopFactoryGenerateStarted)
         gko::log::Logger::linop_factory_generate_started_mask);
     auto factory =
         gko::solver::Bicgstab<>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(3u).on(exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(3u))
             .on(exec);
     auto input = factory->generate(gko::matrix::Dense<>::create(exec));
 
@@ -462,8 +461,7 @@ TEST(Record, CatchesLinopFactoryGenerateCompleted)
         gko::log::Logger::linop_factory_generate_completed_mask);
     auto factory =
         gko::solver::Bicgstab<>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(3u).on(exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(3u))
             .on(exec);
     auto input = factory->generate(gko::matrix::Dense<>::create(exec));
     auto output = factory->generate(gko::matrix::Dense<>::create(exec));
@@ -569,8 +567,7 @@ TEST(Record, CatchesIterations)
         gko::log::Record::create(gko::log::Logger::iteration_complete_mask);
     auto factory =
         gko::solver::Bicgstab<>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(3u).on(exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(3u))
             .on(exec);
     auto solver = factory->generate(gko::initialize<Dense>({1.1}, exec));
     auto right_hand_side = gko::initialize<Dense>({-5.5}, exec);

diff --git a/core/test/log/stream.cpp b/core/test/log/stream.cpp
@@ -606,8 +606,7 @@ TYPED_TEST(Stream, CatchesLinopFactoryGenerateStarted)
         gko::log::Logger::linop_factory_generate_started_mask, out);
     auto factory =
         gko::solver::Bicgstab<TypeParam>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(3u).on(exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(3u))
             .on(exec);
     auto input = factory->generate(gko::matrix::Dense<TypeParam>::create(exec));
     std::stringstream ptrstream_factory;
@@ -633,8 +632,7 @@ TYPED_TEST(Stream, CatchesLinopFactoryGenerateCompleted)
         gko::log::Logger::linop_factory_generate_completed_mask, out);
     auto factory =
         gko::solver::Bicgstab<TypeParam>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(3u).on(exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(3u))
             .on(exec);
     auto input = factory->generate(gko::matrix::Dense<TypeParam>::create(exec));
     auto output =
@@ -815,8 +813,7 @@ TYPED_TEST(Stream, CatchesIterationsWithVerbose)
 
     auto factory =
         gko::solver::Bicgstab<TypeParam>::build()
-            .with_criteria(
-                gko::stop::Iteration::build().with_max_iters(3u).on(exec))
+            .with_criteria(gko::stop::Iteration::build().with_max_iters(3u))
             .on(exec);
     auto solver = factory->generate(gko::initialize<Dense>({1.1}, exec));
     auto right_hand_side = gko::initialize<Dense>({-5.5}, exec);