ParIlu improvements according to reviewer

- Added additional test with more iterations for OpenMP ParIlu test
  (Therefore, creating a function `compute_lu` to simplify that)
- Renaming variables to better fit their purpose

omp/test/factorization/par_ilu_kernels.cpp

@@ -92,8 +92,10 @@ class ParIlu : public ::testing::Test {
  std::shared_ptr<const Csr> csr_ref;
  std::shared_ptr<const Csr> csr_omp;
 
- void compute_nnz(index_type *l_row_ptrs_ref, index_type *u_row_ptrs_ref,
- index_type *l_row_ptrs_omp, index_type *u_row_ptrs_omp)
+ void initialize_row_ptrs(index_type *l_row_ptrs_ref,
+ index_type *u_row_ptrs_ref,
+ index_type *l_row_ptrs_omp,
+ index_type *u_row_ptrs_omp)
  {
  gko::kernels::reference::par_ilu_factorization::initialize_row_ptrs_l_u(
  ref, gko::lend(csr_ref), l_row_ptrs_ref, u_row_ptrs_ref);
@@ -110,10 +112,11 @@ class ParIlu : public ::testing::Test {
  gko::Array<index_type> l_row_ptrs_omp{omp, num_row_ptrs};
  gko::Array<index_type> u_row_ptrs_omp{omp, num_row_ptrs};
 
- compute_nnz(l_row_ptrs_ref.get_data(), u_row_ptrs_ref.get_data(),
- l_row_ptrs_omp.get_data(), u_row_ptrs_omp.get_data());
- // Since `compute_nnz` was already tested, it is expected that `*_ref`
- // and `*_omp` contain identical values
+ initialize_row_ptrs(
+ l_row_ptrs_ref.get_data(), u_row_ptrs_ref.get_data(),
+ l_row_ptrs_omp.get_data(), u_row_ptrs_omp.get_data());
+ // Since `initialize_row_ptrs` was already tested, it is expected that
+ // `*_ref` and `*_omp` contain identical values
  auto l_nnz = l_row_ptrs_ref.get_const_data()[num_row_ptrs - 1];
  auto u_nnz = u_row_ptrs_ref.get_const_data()[num_row_ptrs - 1];
 
@@ -136,6 +139,41 @@ class ParIlu : public ::testing::Test {
  gko::kernels::omp::par_ilu_factorization::initialize_l_u(
  omp, gko::lend(csr_omp), gko::lend(*l_omp), gko::lend(*u_omp));
  }
+
+ template <typename ToType, typename FromType>
+ static std::unique_ptr<ToType> static_unique_ptr_cast(
+ std::unique_ptr<FromType> &&from)
+ {
+ return std::unique_ptr<ToType>{static_cast<ToType *>(from.release())};
+ }
+
+ void compute_lu(std::unique_ptr<Csr> *l_ref, std::unique_ptr<Csr> *u_ref,
+ std::unique_ptr<Csr> *l_omp, std::unique_ptr<Csr> *u_omp,
+ gko::size_type iterations = 0)
+ {
+ auto coo_ref = Coo::create(ref);
+ csr_ref->convert_to(gko::lend(coo_ref));
+ auto coo_omp = Coo::create(omp);
+ csr_omp->convert_to(gko::lend(coo_omp));
+ initialize_lu(l_ref, u_ref, l_omp, u_omp);
+ auto u_transpose_lin_op_ref = (*u_ref)->transpose();
+ auto u_transpose_csr_ref =
+ static_unique_ptr_cast<Csr>(std::move(u_transpose_lin_op_ref));
+ auto u_transpose_lin_op_omp = (*u_omp)->transpose();
+ auto u_transpose_csr_omp =
+ static_unique_ptr_cast<Csr>(std::move(u_transpose_lin_op_omp));
+
+ gko::kernels::reference::par_ilu_factorization::compute_l_u_factors(
+ ref, iterations, gko::lend(coo_ref), gko::lend(*l_ref),
+ gko::lend(u_transpose_csr_ref));
+ gko::kernels::omp::par_ilu_factorization::compute_l_u_factors(
+ omp, iterations, gko::lend(coo_omp), gko::lend(*l_omp),
+ gko::lend(u_transpose_csr_omp));
+ auto u_lin_op_ref = u_transpose_csr_ref->transpose();
+ auto u_lin_op_omp = u_transpose_csr_omp->transpose();
+ *u_ref = static_unique_ptr_cast<Csr>(std::move(u_transpose_csr_ref));
+ *u_omp = static_unique_ptr_cast<Csr>(std::move(u_transpose_csr_omp));
+ }
 };
 
 
@@ -151,7 +189,8 @@ TEST_F(ParIlu, OmpKernelInitializeRowPtrsLUEquivalentToRef)
  auto l_row_ptrs_omp = l_row_ptrs_array_omp.get_data();
  auto u_row_ptrs_omp = u_row_ptrs_array_omp.get_data();
 
- compute_nnz(l_row_ptrs_ref, u_row_ptrs_ref, l_row_ptrs_omp, u_row_ptrs_omp);
+ initialize_row_ptrs(l_row_ptrs_ref, u_row_ptrs_ref, l_row_ptrs_omp,
+ u_row_ptrs_omp);
 
  ASSERT_TRUE(std::equal(l_row_ptrs_ref, l_row_ptrs_ref + num_row_ptrs,
  l_row_ptrs_omp));
@@ -180,26 +219,26 @@ TEST_F(ParIlu, KernelComputeParILUIsEquivalentToRef)
  std::unique_ptr<Csr> u_ref{};
  std::unique_ptr<Csr> l_omp{};
  std::unique_ptr<Csr> u_omp{};
- gko::size_type iterations{};
- auto coo_ref = Coo::create(ref);
- csr_ref->convert_to(gko::lend(coo_ref));
- auto coo_omp = Coo::create(omp);
- csr_omp->convert_to(gko::lend(coo_omp));
- initialize_lu(&l_ref, &u_ref, &l_omp, &u_omp);
- auto u_transpose_lin_op_ref = u_ref->transpose();
- auto u_transpose_ptr_ref = static_cast<Csr *>(u_transpose_lin_op_ref.get());
- auto u_transpose_lin_op_omp = u_omp->transpose();
- auto u_transpose_ptr_omp = static_cast<Csr *>(u_transpose_lin_op_omp.get());
-
- gko::kernels::reference::par_ilu_factorization::compute_l_u_factors(
- ref, iterations, gko::lend(coo_ref), gko::lend(l_ref),
- u_transpose_ptr_ref);
- gko::kernels::omp::par_ilu_factorization::compute_l_u_factors(
- omp, iterations, gko::lend(coo_omp), gko::lend(l_omp),
- u_transpose_ptr_omp);
+
+ compute_lu(&l_ref, &u_ref, &l_omp, &u_omp);
 
  GKO_ASSERT_MTX_NEAR(l_ref, l_omp, 5e-3);
- GKO_ASSERT_MTX_NEAR(u_transpose_ptr_ref, u_transpose_ptr_omp, 5e-3);
+ GKO_ASSERT_MTX_NEAR(u_ref, u_omp, 5e-3);
+}
+
+
+TEST_F(ParIlu, KernelComputeParILUWithMoreIterationsIsEquivalentToRef)
+{
+ std::unique_ptr<Csr> l_ref{};
+ std::unique_ptr<Csr> u_ref{};
+ std::unique_ptr<Csr> l_omp{};
+ std::unique_ptr<Csr> u_omp{};
+ gko::size_type iterations{20};
+
+ compute_lu(&l_ref, &u_ref, &l_omp, &u_omp, iterations);
+
+ GKO_ASSERT_MTX_NEAR(l_ref, l_omp, 1e-14);
+ GKO_ASSERT_MTX_NEAR(u_ref, u_omp, 1e-14);
 }