Revert "use unpack directly from factorization and add unpack with st…

…rategy" to avoid new strategy input

core/factorization/factorization.cpp

@@ -31,25 +31,10 @@ GKO_REGISTER_OPERATION(initialize_l, factorization::initialize_l);
 
 template <typename ValueType, typename IndexType>
 std::unique_ptr<Factorization<ValueType, IndexType>>
-Factorization<ValueType, IndexType>::unpack(
-    std::shared_ptr<typename matrix_type::strategy_type> lower_factor_strategy,
-    std::shared_ptr<typename matrix_type::strategy_type> upper_factor_strategy)
-    const
+Factorization<ValueType, IndexType>::unpack() const
 {
     const auto exec = this->get_executor();
     const auto size = this->get_size();
-    auto create_matrix = [](auto exec, auto size, auto vals, auto col_idxs,
-                            auto row_ptrs, auto strategy) {
-        if (strategy == nullptr) {
-            return matrix_type::create(exec, size, std::move(vals),
-                                       std::move(col_idxs),
-                                       std::move(row_ptrs));
-        } else {
-            return matrix_type::create(exec, size, std::move(vals),
-                                       std::move(col_idxs), std::move(row_ptrs),
-                                       strategy);
-        }
-    };
     switch (this->get_storage_type()) {
     case storage_type::empty:
         GKO_NOT_SUPPORTED(nullptr);
@@ -68,14 +53,12 @@ Factorization<ValueType, IndexType>::unpack(
         const auto u_nnz =
             static_cast<size_type>(get_element(u_row_ptrs, size[0]));
         // create matrices
-        auto l_mtx =
-            create_matrix(exec, size, array<value_type>{exec, l_nnz},
-                          array<index_type>{exec, l_nnz}, std::move(l_row_ptrs),
-                          lower_factor_strategy);
-        auto u_mtx =
-            create_matrix(exec, size, array<value_type>{exec, u_nnz},
-                          array<index_type>{exec, u_nnz}, std::move(u_row_ptrs),
-                          upper_factor_strategy);
+        auto l_mtx = matrix_type::create(
+            exec, size, array<value_type>{exec, l_nnz},
+            array<index_type>{exec, l_nnz}, std::move(l_row_ptrs));
+        auto u_mtx = matrix_type::create(
+            exec, size, array<value_type>{exec, u_nnz},
+            array<index_type>{exec, u_nnz}, std::move(u_row_ptrs));
         // fill matrices
         exec->run(make_initialize_l_u(mtx.get(), l_mtx.get(), u_mtx.get()));
         return create_from_composition(
@@ -89,10 +72,9 @@ Factorization<ValueType, IndexType>::unpack(
         const auto l_nnz =
             static_cast<size_type>(get_element(l_row_ptrs, size[0]));
         // create matrices
-        auto l_mtx =
-            create_matrix(exec, size, array<value_type>{exec, l_nnz},
-                          array<index_type>{exec, l_nnz}, std::move(l_row_ptrs),
-                          lower_factor_strategy);
+        auto l_mtx = matrix_type::create(
+            exec, size, array<value_type>{exec, l_nnz},
+            array<index_type>{exec, l_nnz}, std::move(l_row_ptrs));
         // fill matrices
         exec->run(make_initialize_l(mtx.get(), l_mtx.get(), false));
         auto u_mtx = l_mtx->conj_transpose();

core/factorization/ilu.cpp

@@ -80,6 +80,7 @@ std::unique_ptr<Composition<ValueType>> Ilu<ValueType, IndexType>::generate_l_u(
     // Converts the system matrix to CSR.
     // Throws an exception if it is not convertible.
     auto local_system_matrix = share(matrix_type::create(exec));
+    std::shared_ptr<const matrix_type> ilu;
     as<ConvertibleTo<matrix_type>>(system_matrix.get())
         ->convert_to(local_system_matrix);
 
@@ -103,26 +104,25 @@ std::unique_ptr<Composition<ValueType>> Ilu<ValueType, IndexType>::generate_l_u(
                 make_const_array_view(
                     exec, local_system_matrix->get_size()[0] + 1,
                     local_system_matrix->get_const_row_ptrs())));
-        auto unpack =
+        ilu =
             gko::experimental::factorization::Lu<ValueType, IndexType>::build()
                 .with_has_all_fillin(false)
                 .with_symbolic_factorization(sparsity)
                 .on(exec)
                 ->generate(local_system_matrix)
-                ->unpack(parameters_.l_strategy, parameters_.u_strategy);
-        return Composition<ValueType>::create(unpack->get_lower_factor(),
-                                              unpack->get_upper_factor());
+                ->get_combined();
+    } else {
+        exec->run(
+            ilu_factorization::make_compute_ilu(local_system_matrix.get()));
+        ilu = local_system_matrix;
     }
-    exec->run(ilu_factorization::make_compute_ilu(local_system_matrix.get()));
-
     // Separate L and U factors: nnz
-    const auto matrix_size = local_system_matrix->get_size();
+    const auto matrix_size = ilu->get_size();
     const auto num_rows = matrix_size[0];
     array<IndexType> l_row_ptrs{exec, num_rows + 1};
     array<IndexType> u_row_ptrs{exec, num_rows + 1};
     exec->run(ilu_factorization::make_initialize_row_ptrs_l_u(
-        local_system_matrix.get(), l_row_ptrs.get_data(),
-        u_row_ptrs.get_data()));
+        ilu.get(), l_row_ptrs.get_data(), u_row_ptrs.get_data()));
 
     // Get nnz from device memory
     auto l_nnz = static_cast<size_type>(get_element(l_row_ptrs, num_rows));
@@ -141,8 +141,8 @@ std::unique_ptr<Composition<ValueType>> Ilu<ValueType, IndexType>::generate_l_u(
         std::move(u_row_ptrs), parameters_.u_strategy);
 
     // Separate L and U: columns and values
-    exec->run(ilu_factorization::make_initialize_l_u(
-        local_system_matrix.get(), l_factor.get(), u_factor.get()));
+    exec->run(ilu_factorization::make_initialize_l_u(ilu.get(), l_factor.get(),
+                                                     u_factor.get()));
 
     return Composition<ValueType>::create(std::move(l_factor),
                                           std::move(u_factor));

include/ginkgo/core/factorization/factorization.hpp

@@ -88,21 +88,10 @@ class Factorization : public EnableLinOp<Factorization<ValueType, IndexType>> {
      * for triangular solves to a composition representation that can also be
      * used to access individual factors and multiply with the factorization.
      *
-     * @param lower_factor_strategy  the Csr strategy for the lower factor and
-     *                               the transposed lower factor.
-     * @param upper_factor_strategy  the Csr strategy for the upper factor
-     *
      * @return  a new Factorization object containing this factorization
      *          represented as storage_type::composition.
-     *
-     * @note The strategy only has effect when it is unpacked from the combined
-     * matrix.
      */
-    std::unique_ptr<Factorization> unpack(
-        std::shared_ptr<typename matrix_type::strategy_type>
-            lower_factor_strategy = nullptr,
-        std::shared_ptr<typename matrix_type::strategy_type>
-            upper_factor_strategy = nullptr) const;
+    std::unique_ptr<Factorization> unpack() const;
 
     /** Returns the storage type used by this factorization. */
     storage_type get_storage_type() const;

reference/test/factorization/factorization.cpp

@@ -252,31 +252,6 @@ TYPED_TEST(Factorization, UnpackCombinedLUWorks)
 }
 
 
-TYPED_TEST(Factorization, UnpackCombinedLUWorksWithStrategy)
-{
-    using factorization_type = typename TestFixture::factorization_type;
-    using matrix_type = typename TestFixture::matrix_type;
-    auto fact = factorization_type::create_from_combined_lu(
-        this->combined_mtx->clone());
-
-    auto separated =
-        fact->unpack(std::make_shared<typename matrix_type::classical>(),
-                     std::make_shared<typename matrix_type::merge_path>());
-
-    ASSERT_EQ(separated->get_storage_type(),
-              gko::experimental::factorization::storage_type::composition);
-    ASSERT_EQ(separated->get_combined(), nullptr);
-    ASSERT_EQ(separated->get_diagonal(), nullptr);
-    GKO_ASSERT_MTX_NEAR(separated->get_lower_factor(), this->lower_mtx, 0.0);
-    GKO_ASSERT_MTX_NEAR(separated->get_upper_factor(), this->upper_nonunit_mtx,
-                        0.0);
-    ASSERT_EQ(separated->get_lower_factor()->get_strategy()->get_name(),
-              "classical");
-    ASSERT_EQ(separated->get_upper_factor()->get_strategy()->get_name(),
-              "merge_path");
-}
-
-
 TYPED_TEST(Factorization, UnpackSymmCombinedCholeskyWorks)
 {
     using matrix_type = typename TestFixture::matrix_type;
@@ -298,35 +273,6 @@ TYPED_TEST(Factorization, UnpackSymmCombinedCholeskyWorks)
 }
 
 
-TYPED_TEST(Factorization, UnpackSymmCombinedCholeskyWorksWithStrategy)
-{
-    using matrix_type = typename TestFixture::matrix_type;
-    using factorization_type = typename TestFixture::factorization_type;
-    auto fact = factorization_type::create_from_combined_cholesky(
-        this->combined_mtx->clone());
-
-    // second one is ignored in cholesky to keep the same behavior as
-    // factorization::Ic
-    auto separated =
-        fact->unpack(std::make_shared<typename matrix_type::classical>(),
-                     std::make_shared<typename matrix_type::merge_path>());
-
-    ASSERT_EQ(separated->get_storage_type(),
-              gko::experimental::factorization::storage_type::symm_composition);
-    ASSERT_EQ(separated->get_combined(), nullptr);
-    ASSERT_EQ(separated->get_diagonal(), nullptr);
-    GKO_ASSERT_MTX_NEAR(separated->get_lower_factor(), this->lower_cholesky_mtx,
-                        0.0);
-    GKO_ASSERT_MTX_NEAR(
-        separated->get_upper_factor(),
-        gko::as<matrix_type>(this->lower_cholesky_mtx->conj_transpose()), 0.0);
-    ASSERT_EQ(separated->get_lower_factor()->get_strategy()->get_name(),
-              "classical");
-    ASSERT_EQ(separated->get_upper_factor()->get_strategy()->get_name(),
-              "classical");
-}
-
-
 TYPED_TEST(Factorization, UnpackCompositionWorks)
 {
     using factorization_type = typename TestFixture::factorization_type;