Avoid symmetric MPI_Igatherv

greole · greole · commit 28ee971b391c · 2025-09-25T17:15:35.000+02:00
diff --git a/include/OGL/.Preconditioner.hpp.swn b/include/OGL/.Preconditioner.hpp.swn
diff --git a/include/OGL/.Preconditioner.hpp.swo b/include/OGL/.Preconditioner.hpp.swo
diff --git a/include/OGL/CommunicationPattern.hpp b/include/OGL/CommunicationPattern.hpp
@@ -19,8 +19,13 @@ struct AllToAllPattern {
     std::vector<int> recv_offsets;
 };
 
-/* @brief computes AllToAllPattern for repart comm from global allToAll pattern
+/* @brief computes AllToAllPattern for repartioned communincator from global
+ * allToAll pattern by discarding all zero communication before and after the
+ * repartioner scope.
  *
+ * @param exec_handler The executor handler
+ * @param allToAll The original allToAll pattern
+ * @param start_rank the original comm_world rank
  */
 AllToAllPattern compute_repart_allToall(const ExecutorHandler &exec_handler,
                                         const AllToAllPattern allToAll,
diff --git a/include/OGL/DevicePersistent/ExecutorHandler.hpp b/include/OGL/DevicePersistent/ExecutorHandler.hpp
@@ -311,7 +311,10 @@ class ExecutorHandler
 
     label get_ranks_per_gpu() const { return device_id_handler_.ranks_per_gpu; }
 
-    void set_ranks_per_gpu(label ranks_per_gpu) { device_id_handler_.ranks_per_gpu= ranks_per_gpu; }
+    void set_ranks_per_gpu(label ranks_per_gpu)
+    {
+        device_id_handler_.ranks_per_gpu = ranks_per_gpu;
+    }
 
     label get_owner_rank() const { return device_id_handler_.global_owner(); }
 
diff --git a/include/OGL/DevicePersistent/Vector.hpp b/include/OGL/DevicePersistent/Vector.hpp
@@ -171,7 +171,7 @@ class PersistentVector
 
     /** Copies the content of the distributed vector back to the original source
      **/
-    MPI_Request copy_back()
+    void copy_back()
     {
         auto exec = exec_.get_device_exec();
         auto rank = exec_.get_host_rank();
@@ -197,43 +197,26 @@ class PersistentVector
         // 	repartAllToAll.recv_counts[0] = 0;
         // }
 
+        // NOTE instead of all_to_all_v based communication MPI_Iscatterv
+        // seems to be prefarable
         // communicate_values(exec, ref_exec, comm, comm_pattern,
         //                    get_vector()->get_local_values(),
         //                    const_cast<T *>(memory_), host_buffer);
-        // std::cout << __FILE__
-        // 	<< " owner_rank " << exec_.get_owner_rank()
-        // 	<< " rank " << Pstream::myProcNo()
-        // 	<< " comm_pattern.send_counts: " << comm_pattern.send_counts
-        // 	<< " comm_pattern.recv_counts: " << comm_pattern.recv_counts
-        // 	<< " send_counts: " << repartAllToAll.send_counts
-        // 	<< " send_offsets: " << repartAllToAll.send_offsets
-        // 	<< " recv_counts: " << repartAllToAll.recv_counts
-        //        	<< "\n";
-
-        // auto start_rep = std::chrono::steady_clock::now();
+
         label send_size = comm_pattern.send_offsets.back();
         auto send_view = gko::array<scalar>::const_view(
             exec, send_size, get_vector()->get_local_values());
         auto tmp = gko::array<scalar>(exec, send_size);
 
         tmp = send_view;
         tmp.set_executor(ref_exec);
-        // auto end_rep = std::chrono::steady_clock::now();
-        // auto delta_t_rep =
-        // std::chrono::duration_cast<std::chrono::microseconds>(end_rep -
-        // start_rep).count() /1000.0; std::cout << __FILE__ << " copy back: "
-        // << delta_t_rep << " [ms]\n";
-
 
         MPI_Request copy_back_req;
-        MPI_Iscatterv(tmp.get_data(),
-                      // get_vector()->get_local_values(),
-                      repartAllToAll.send_counts.data(),
+        MPI_Iscatterv(tmp.get_data(), repartAllToAll.send_counts.data(),
                       repartAllToAll.send_offsets.data(), MPI_DOUBLE,
                       const_cast<T *>(memory_), repartAllToAll.recv_counts[0],
                       MPI_DOUBLE, 0, repart_comm->get(), &copy_back_req);
         MPI_Wait(&copy_back_req, MPI_STATUS_IGNORE);
-        return copy_back_req;
     }
 
     /** Writes the content of the distributed vector to disk
diff --git a/include/OGL/lduLduBase.hpp b/include/OGL/lduLduBase.hpp
@@ -306,15 +306,8 @@ class lduLduBase : public OGL_Info,
             delta_t_solve_ = delta_t_solve;
         }
 
-        // auto start_rep = std::chrono::steady_clock::now();
         TIME_WITH_FIELDNAME(verbose_, copy_x_back, this->fieldName(),
                             dist_x.copy_back();)
-        // auto copy_back_req = dist_x.copy_back();
-        // auto end_rep = std::chrono::steady_clock::now();
-        // auto delta_t_rep =
-        // std::chrono::duration_cast<std::chrono::microseconds>(end_rep -
-        // start_rep).count() /1000.0; std::cout << __FILE__ << " full copy
-        // back: " << delta_t_rep << " [ms]\n";
 
         auto bandwidth_copy_back =
             sizeof(scalar) * psi.size() / delta_t_copy_x_back / 1000.0;
@@ -346,8 +339,6 @@ class lduLduBase : public OGL_Info,
         std::to_string(bandwidth_copy_back) + std::string(" [GByte/s]");
         MLOG_0(verbose_, msg)
 
-        // MPI_Wait(&copy_back_req,MPI_STATUS_IGNORE);
-
         return solverPerf;
     }
 
diff --git a/src/MatrixWrapper/Distributed.cpp b/src/MatrixWrapper/Distributed.cpp
@@ -121,13 +121,15 @@ void generate_alltoall_update_data(
     std::vector<RepartDistMatrix::all_to_all_data> &update_data)
 {
     label linop_offset_store{0};
-    for (size_t i = 0; i < 3; i++) {
+    // NOTE in case of symmetric matrix 0 (upper) is same as 1 (lower)
+    // thus we can start at 1
+    label start = 0;
+    for (size_t i = start; i < 3; i++) {
         label interface_size = in->get_rows()[i].size();
         label linop_idx = (fuse) ? 0 : in->get_id()[i];
         label linop_offset = (fuse) ? linop_offset_store : 0;
         auto comm_pattern = compute_gather_to_owner_counts(
             exec_handler, ranks_per_owner, interface_size);
-
         size_t recv_size = comm_pattern.recv_offsets.back();
 
         // NOTE Probably dont need to store linops[linop-idx] because we can
@@ -395,6 +397,9 @@ void update_impl(
     auto all_to_all_update = [repart_comm, ref_exec, device_exec,
                               all_to_all_update_data, host_A, force_host_buffer,
                               exec_handler, rank]() {
+        // NOTE if symmetric (get it from host_A) we can skip id=0 and wait till
+        // id=1 has been copied to use device copy
+        //
         for (auto [id, comm_pattern, data_ptr] : all_to_all_update_data) {
             // auto start = std::chrono::steady_clock::now();
             auto repartAllToAll =
@@ -409,24 +414,43 @@ void update_impl(
             // communicate_values(ref_exec, device_exec, repart_comm,
             // repartAllToAll,
             //                    send_data_ptr, data_ptr, force_host_buffer);
-            // if ( repart_comm->rank() == 0 ) {
             // std::cout << __FILE__ <<
             //     " Pstream::rank " << Pstream::myProcNo() <<
             //     " repart_rank() "  << repart_comm->rank() <<
             //     " send_offsets.back() "  <<
+            //     " id " << id <<
             //     repartAllToAll.send_offsets.back() << " recv_counts: "  <<
             //     repartAllToAll.recv_counts << " recv_offsets: "  <<
             //     repartAllToAll.recv_offsets <<
             // std::endl;
-            // }
-            MPI_Request request;
-
-            MPI_Igatherv(send_data_ptr, repartAllToAll.send_offsets.back(),
-                         MPI_DOUBLE, data_ptr,
-                         repartAllToAll.recv_counts.data(),
-                         repartAllToAll.recv_offsets.data(), MPI_DOUBLE, 0,
-                         repart_comm->get(), &request);
-            MPI_Wait(&request, MPI_STATUS_IGNORE);
+
+            if (id == 0 && host_A->get_symmetric()) {
+            } else {
+                MPI_Request request;
+                MPI_Igatherv(send_data_ptr, repartAllToAll.send_offsets.back(),
+                             MPI_DOUBLE, data_ptr,
+                             repartAllToAll.recv_counts.data(),
+                             repartAllToAll.recv_offsets.data(), MPI_DOUBLE, 0,
+                             repart_comm->get(), &request);
+                MPI_Wait(&request, MPI_STATUS_IGNORE);
+            }
+
+            // Perform symmetric inter device copy
+            if (id == 1 && repart_comm->rank() == 0 &&
+                host_A->get_symmetric()) {
+                auto [zid, zcomm_pattern, zdata_ptr] =
+                    all_to_all_update_data[0];
+                // copy recv size data from data_ptr to zdata_ptr
+                //
+                label recv_buffer_size = repartAllToAll.recv_offsets.back();
+                auto l_view = gko::array<scalar>::view(
+                    device_exec, recv_buffer_size, data_ptr);
+
+                auto u_view = gko::array<scalar>::view(
+                    device_exec, recv_buffer_size, zdata_ptr);
+
+                u_view = l_view;
+            }
         }
     };
 
diff --git a/src/StoppingCriterion.cpp b/src/StoppingCriterion.cpp
@@ -53,9 +53,6 @@ StoppingCriterion::OpenFOAMDistStoppingCriterion::compute_normfactor_dist(
                              end_axref - start_axref)
                              .count() /
                          1.0;
-    // std::cout << __FILE__ << " delta_t_axref " << delta_t_axref << " [mu
-    // s]\n";
-
     auto unity =
         gko::initialize<gko::matrix::Dense<scalar>>(1, {1.0}, device_exec);
 
@@ -177,7 +174,6 @@ bool StoppingCriterion::OpenFOAMDistStoppingCriterion::check_impl(
                               end_eval - start_eval)
                               .count() /
                           1.0;
-    // std::cout << __FILE__ << "time " << *(parameters_.time) << " [mu s]\n";
     return result;
 }
 
diff --git a/test/unit/MatrixWrapper/Distributed.cpp b/test/unit/MatrixWrapper/Distributed.cpp
@@ -324,13 +324,13 @@ TEST_P(DistMatL2D, canApplyCorrectly)
 
     // Act
     bool active = repartitioner->get_repart_size() != 0;
-    if (active){
-    distributed->apply(b, x);
-    auto res_x = std::vector<scalar>(
-        x->get_local_vector()->get_const_values(),
-        x->get_local_vector()->get_const_values() + local_vec_dim[0]);
+    if (active) {
+        distributed->apply(b, x);
+        auto res_x = std::vector<scalar>(
+            x->get_local_vector()->get_const_values(),
+            x->get_local_vector()->get_const_values() + local_vec_dim[0]);
 
-    ASSERT_EQ(res_x, exp_x[name][fused][ranks_per_gpu][rank]);
+        ASSERT_EQ(res_x, exp_x[name][fused][ranks_per_gpu][rank]);
     }
 }
 
