This fails

Grid/algorithms/multigrid/RecursiveSchurInverse.h

@@ -132,7 +132,15 @@ public:
   deviceVector<int64_t>  dOwnerOff;    // panel row -> owner's first panel row
   deviceVector<int64_t>  dOwnerRows;   // panel row -> owner's row count
 
-  // DENSE_GATHER            : 1 => AllGatherV in place of zero-fill+GlobalSum
+  // DENSE_GATHER            : transport for the panel assembly.
+  //     0 = zero-fill + GlobalSumVector  (default; moves the payload twice)
+  //     1 = MPI_Allgatherv               (KNOWN BROKEN here, see guard below)
+  //     2 = C sequential MPI_Bcast, one per contributing rank.  Bcast has no
+  //         count vector, so the zero-count shape that breaks Allgatherv
+  //         cannot arise.  Costs C collectives instead of 1 and the roots do
+  //         not transmit concurrently, so the byte cost is ~2*panel per rank
+  //         -- the same as the allreduce.  It wins only if Bcast is faster
+  //         PER BYTE than Allreduce, which is why it must be measured.
   // DENSE_GATHER_MIN_BYTES  : panels below this stay on the allreduce path.
   //                           Default 0 (always gather).  The Schur tree puts
   //                           94% of its bytes in panels with ~3.7 MB per-rank
@@ -249,7 +257,7 @@ public:
     // Fail here, in the first second, rather than 100 s and 36 nodes into a
     // job.  DENSE_GATHER_FORCE=1 proceeds anyway for debugging.
     ///////////////////////////////////////////////////////////////////////
-    if ( useGather && !getenv("DENSE_GATHER_FORCE") )
+    if ( (useGather==1) && !getenv("DENSE_GATHER_FORCE") )
     {
       std::cout << GridLogError
                 << "DENSE_GATHER=1 is disabled: MPI_Allgatherv on this MPI is"
@@ -407,9 +415,38 @@ public:
                     << "  rank0: count "<<counts[me]<<(owner?" owner":" non-owner")
                     << std::endl;
         }
-        void *send = owner ? (void *)B.ColumnWindow(colB+j0) : (void *)&dRecvBuf[0];
-        grid->AllGatherV(send, counts[me],
-                         (void *)&dRecvBuf[0], counts, displs, sizeof(ComplexD));
+        if ( gatherThis == 1 )
+        {
+          void *send = owner ? (void *)B.ColumnWindow(colB+j0) : (void *)&dRecvBuf[0];
+          grid->AllGatherV(send, counts[me],
+                           (void *)&dRecvBuf[0], counts, displs, sizeof(ComplexD));
+        }
+        else
+        {
+          ////////////////////////////////////////////////////////////////
+          // C sequential broadcasts into the SAME rank-major staging
+          // buffer, so the repack below is shared with the AllGatherV arm.
+          // The root must already hold its own block: B.ColumnWindow is
+          // contiguous rows_r x nchunk, and so is its slot in dRecvBuf.
+          // Every rank issues all C broadcasts in the same order, so
+          // collective matching stays positional and safe.
+          ////////////////////////////////////////////////////////////////
+          if ( owner )
+          {
+            int64_t off_me = rowStart[me] - rowStart[rB0];
+            acceleratorCopyDeviceToDevice((void *)B.ColumnWindow(colB+j0),
+                                          (void *)&dRecvBuf[off_me*nchunk],
+                                          (uint64_t)B.rows*nchunk*sizeof(ComplexD));
+          }
+          for(int r=rB0;r<rB1;r++)
+          {
+            int64_t off_r  = rowStart[r]   - rowStart[rB0];
+            int64_t rows_r = rowStart[r+1] - rowStart[r];
+            if ( rows_r )
+              grid->Broadcast(r,(void *)&dRecvBuf[off_r*nchunk],
+                              (uint64_t)rows_r*nchunk*sizeof(ComplexD));
+          }
+        }
         tar += usecond();
 
         tRepack -= usecond();

Grid/util/Init.cc

@@ -654,17 +654,19 @@ void Grid_init(int *argc,char ***argv)
 void Grid_finalize(void)
 {
   std::cout<<GridLogMessage<<"*******************************************"<<std::endl;
-  std::cout<<GridLogMessage<<"******* Grid Finalize                ******"<<std::endl;
-  std::cout<<GridLogMessage<<"*******************************************"<<std::endl;
-
+  std::cout<<GridLogMessage<<"******* Grid Finalize entered         ******"<<std::endl;
+  CartesianCommunicator::BarrierWorld();
+  std::cout<<GridLogMessage<<"******* Grid Finalize synched         ******"<<std::endl;
+				    
 #if defined (GRID_COMMS_MPI) || defined (GRID_COMMS_MPI3) || defined (GRID_COMMS_MPIT)
-  MPI_Barrier(MPI_COMM_WORLD);
   MPI_Finalize();
   Grid_unquiesce_nodes();
 #endif
 #if defined (GRID_COMMS_SHMEM)
   shmem_finalize();
 #endif
+  std::cout<<GridLogMessage<<"******* Grid Finalize successful     ******"<<std::endl;
+  std::cout<<GridLogMessage<<"*******************************************"<<std::endl;
   Grid_is_initialised = 0;
 }
 

tests/debug/Test_allgather.cc

@@ -208,8 +208,15 @@ int main(int argc, char **argv)
   //   18.7 MB gathered).  Depth 0 is AG_NPART=144 AG_WORDS=232800.
   const int64_t AGW = getenv("AG_WORDS") ? atol(getenv("AG_WORDS")) : 4096;
   const int     AGN = getenv("AG_NPART") ? atoi(getenv("AG_NPART")) : (P+1)/2;
+  // Stage skips.  T6 is the pathological one: at 288 ranks with 9 contributors
+  // it takes ~26 minutes and aborts on at least one rank, which would prevent
+  // T7 from ever running.  T7 is therefore ordered FIRST, and AG_T6=0 skips
+  // the slow stage entirely when only the Bcast answer is wanted.
+  const int     doT5 = getenv("AG_T5") ? atoi(getenv("AG_T5")) : 1;
+  const int     doT6 = getenv("AG_T6") ? atoi(getenv("AG_T6")) : 1;
+  const int     doT7 = getenv("AG_T7") ? atoi(getenv("AG_T7")) : 1;
 
-  {
+  if ( doT5 ) {
     const int64_t W = AGW;                        // words per contributing rank
     std::vector<int> counts(P,0), displs(P,0);
     int64_t total=0;
@@ -233,7 +240,42 @@ int main(int argc, char **argv)
            std::to_string(total*16/1048576)+" MB gathered", ok);
   }
 
-  {
+  ////////////////////////////////////////////////////////////////////////
+  // T7 : the SAME shape as T6, but assembled by C sequential MPI_Bcast --
+  // one broadcast per contributing rank -- instead of one MPI_Allgatherv.
+  //
+  // This is the transport of DENSE_GATHER=2.  Bcast takes no count vector,
+  // so the zero-count asymmetry that makes T6 run at ~0.18 MB/s and trip
+  // mpir_request.h:508 cannot arise.  It costs C collectives rather than 1
+  // and the roots do not transmit concurrently, so the byte cost is about
+  // 2x that of an ideal allgather; it is worth having only if Bcast is
+  // faster PER BYTE than the alternatives.  Compare the T6 and T7 times.
+  ////////////////////////////////////////////////////////////////////////
+  if ( doT7 ) {
+    const int64_t W = AGW;
+    int nroot = AGN < 1 ? 1 : (AGN > P ? P : AGN);
+    int64_t total = (int64_t)nroot*W;
+
+    deviceVector<ComplexD> dall(total>0?total:1);
+    std::vector<ComplexD>  hall(total), hmine(W);
+    for(int64_t i=0;i<W;i++) hmine[i]=Stamp(me,i);
+    if ( me < nroot )                                  // roots seed their slot
+      acceleratorCopyToDevice(&hmine[0],&dall[(int64_t)me*W],W*sizeof(ComplexD));
+
+    for(int r=0;r<nroot;r++)
+      grid->Broadcast(r,(void *)&dall[(int64_t)r*W],(uint64_t)W*sizeof(ComplexD));
+
+    acceleratorCopyFromDevice(&dall[0],&hall[0],total*sizeof(ComplexD));
+    bool ok=true;
+    for(int r=0;r<nroot;r++)
+      for(int64_t i=0;i<W;i++)
+        if ( hall[(int64_t)r*W+i] != Stamp(r,i) ) ok=false;
+    Report("T7  "+std::to_string(nroot)+" x MPI_Bcast on DEVICE, "+
+           std::to_string(W*16/1024)+" KB each, "+
+           std::to_string(total*16/1048576)+" MB assembled", ok);
+  }
+
+  if ( doT6 ) {
     // Only the first half of the ranks contribute; the rest send count 0.
     const int64_t W = AGW;
     int half = AGN < 1 ? 1 : (AGN > P ? P : AGN);
@@ -262,6 +304,19 @@ int main(int argc, char **argv)
            std::to_string(total*16/1048576)+" MB gathered", ok);
   }
 
+  // Reduce across ranks before reporting.  Report() prints from rank 0 only
+  // and `failures` is rank-local, so a stage that passes on rank 0 and fails
+  // (or aborts) elsewhere would otherwise be announced as ALL PASS -- which
+  // is exactly what happened at 288 ranks with 9 contributors.
+  {
+    uint64_t f = failures;
+    grid->GlobalSum(f);
+    if ( f && !failures )
+      std::cout << GridLogMessage << "  ** failures on OTHER ranks: " << f
+                << " (this rank saw none) **" << std::endl;
+    failures = (int)f;
+  }
+
   std::cout << GridLogMessage << (failures ? "AllGather regression: FAILURES" 
                                            : "AllGather regression: ALL PASS") << std::endl;