Review update.

+ Ensure proper use of `HAS_REFERENCE`.
+ Move to `exec_ref` in all cases
+ Improve code readability and add a few documentation.

Co-authored-by: Yuhsiang Tsai <yhmtsai@gmail.com>

test_install/CMakeLists.txt

@@ -43,11 +43,12 @@ if(GINKGO_BUILD_CUDA)
             ginkgo_switch_to_windows_static("CUDA")
         endif()
     endif()
-    set_source_files_properties(test_install.cpp PROPERTIES LANGUAGE CUDA)
-    add_executable(test_install_cuda test_install.cpp)
+    configure_file(test_install.cpp test_install.cu COPYONLY)
+    add_executable(test_install_cuda test_install.cu)
     target_compile_options(test_install_cuda
         PRIVATE "$<$<COMPILE_LANGUAGE:CUDA>:${GINKGO_CUDA_ARCH_FLAGS}>")
     target_compile_definitions(test_install_cuda PRIVATE HAS_CUDA=1)
+    target_compile_definitions(test_install_cuda PRIVATE HAS_REFERENCE=1)
     target_link_libraries(test_install_cuda PRIVATE Ginkgo::ginkgo)
 endif()
 
@@ -70,6 +71,7 @@ if(GINKGO_BUILD_HIP)
 
     target_link_libraries(test_install_hip PRIVATE Ginkgo::ginkgo)
     target_compile_definitions(test_install_hip PRIVATE HAS_HIP=1)
+    target_compile_definitions(test_install_hip PRIVATE HAS_REFERENCE=1)
 
     # If we always link with CXX there is no RPATH issue
     set_target_properties(test_install_hip PROPERTIES LINKER_LANGUAGE CXX)

test_install/test_install.cpp

@@ -69,12 +69,13 @@ void check_spmv(std::shared_ptr<gko::Executor> exec,
     test->apply(b, gko::lend(x_clone));
 
 #if defined(HAS_HIP) || defined(HAS_CUDA)
-    auto test_ref = Mtx::create(exec->get_master());
-    auto x_ref = gko::clone(exec->get_master(), x);
+    auto exec_ref = exec->get_master();
+    auto test_ref = Mtx::create(exec_ref);
+    auto x_ref = gko::clone(exec_ref, x);
     test_ref->read(A_raw);
     test_ref->apply(b, gko::lend(x_ref));
 
-    auto x_clone_ref = gko::clone(exec->get_master(), gko::lend(x_clone));
+    auto x_clone_ref = gko::clone(exec_ref, gko::lend(x_clone));
     assert_similar_matrices(gko::lend(x_clone_ref), gko::lend(x_ref), 1e-14);
 #endif  // defined(HAS_HIP) || defined(HAS_CUDA)
 #endif  // HAS_REFERENCE
@@ -89,9 +90,6 @@ void check_solver(std::shared_ptr<gko::Executor> exec,
     using Mtx = gko::matrix::Csr<>;
     auto A =
         gko::share(Mtx::create(exec, std::make_shared<Mtx::load_balance>()));
-#if HAS_REFERENCE
-    A->read(A_raw);
-#endif  // HAS_REFERENCE
 
     auto num_iters = 20u;
     double reduction_factor = 1e-7;
@@ -105,27 +103,32 @@ void check_solver(std::shared_ptr<gko::Executor> exec,
                     .on(exec))
             .on(exec);
 #if HAS_REFERENCE
+    A->read(A_raw);
     auto x_clone = gko::clone(x);
     solver_gen->generate(A)->apply(b, gko::lend(x_clone));
+    // x_clone has the device result if using HIP or CUDA, otherwise it is
+    // reference only
 
 #if defined(HAS_HIP) || defined(HAS_CUDA)
+    // If we are on a device, we need to run a reference test to compare against
+    auto exec_ref = exec->get_master();
     auto A_ref =
-        gko::share(Mtx::create(exec, std::make_shared<Mtx::load_balance>()));
+        gko::share(Mtx::create(exec_ref, std::make_shared<Mtx::load_balance>()));
     A_ref->read(A_raw);
-    auto refExec = exec->get_master();
     auto solver_gen_ref =
         Solver::build()
             .with_criteria(
                 gko::stop::Iteration::build().with_max_iters(num_iters).on(
-                    exec),
+                    exec_ref),
                 gko::stop::ResidualNorm<>::build()
                     .with_reduction_factor(reduction_factor)
-                    .on(refExec))
-            .on(refExec);
-    auto x_ref = gko::clone(exec->get_master(), x);
+                    .on(exec_ref))
+            .on(exec_ref);
+    auto x_ref = gko::clone(exec_ref, x);
     solver_gen->generate(A_ref)->apply(b, gko::lend(x_ref));
 
-    auto x_clone_ref = gko::clone(exec->get_master(), gko::lend(x_clone));
+    // Actually check that `x_clone` is similar to `x_ref`
+    auto x_clone_ref = gko::clone(exec_ref, gko::lend(x_clone));
     assert_similar_matrices(gko::lend(x_clone_ref), gko::lend(x_ref), 1e-12);
 #endif  // defined(HAS_HIP) || defined(HAS_CUDA)
 #endif  // HAS_REFERENCE