fix allgather for empty buffers (facebookincubator#179) (#356)

Summary:
I used PyTorch-Ignite metrics that were all-gathered on both NCCL and Gloo backends.
In one of the cases the tensor (buffer) was empty and while NCCL tolerated this (hopefully as a no-op), Gloo gave me SIGFPE(8). I think this is because of the modulo by `dataSize` present today in Gloo all-gather code paths.
I believe this can also be a no-op for Gloo, especially since contextSize(rank)=1 is already a no-op.

Pull Request resolved: #356

Reviewed By: albanD, atalman

Differential Revision: D45153246

Pulled By: malfet

fbshipit-source-id: 135165158eaad2dca591f6cc3ccec52edca38912

Author: Alexander Jipa

gloo/allgather.cc

@@ -53,8 +53,8 @@ void allgather(AllgatherOptions& opts) {
         in->size);
   }
 
-  // Short circuit if there is only a single process.
-  if (context->size == 1) {
+  // Short circuit if there is only a single process or the output is empty.
+  if (context->size == 1 || outBytes == 0) {
     return;
   }
 

gloo/allgather_ring.h

@@ -55,6 +55,10 @@ class AllgatherRing : public Algorithm {
   virtual ~AllgatherRing() {}
 
   void run() {
+    // Short circuit if there is only a single process or the output is empty.
+    if (this->contextSize_ == 1 || count_ == 0) {
+      return;
+    }
     const int rank = this->contextRank_;
     const int numRounds = this->contextSize_ - 1;
 

gloo/allgatherv.cc

@@ -111,8 +111,8 @@ void allgatherv(AllgathervOptions& opts) {
     }
   }
 
-  // Short circuit if there is only a single process.
-  if (context->size == 1) {
+  // Short circuit if there is only a single process or the output is empty.
+  if (context->size == 1 || offset == 0) {
     return;
   }
 

gloo/test/allgather_test.cc

@@ -100,7 +100,7 @@ INSTANTIATE_TEST_CASE_P(
     ::testing::Combine(
         ::testing::ValuesIn(kTransportsForClassAlgorithms),
         ::testing::Range(2, 10),
-        ::testing::Values(4, 100, 1000, 10000),
+        ::testing::Values(0, 4, 100, 1000, 10000),
         ::testing::Range(1, 4)));
 
 using NewParam = std::tuple<Transport, int, int, bool>;
@@ -157,7 +157,7 @@ INSTANTIATE_TEST_CASE_P(
     ::testing::Combine(
         ::testing::ValuesIn(kTransportsForFunctionAlgorithms),
         ::testing::Values(1, 2, 4, 7),
-        ::testing::Values(4, 100, 1000, 10000),
+        ::testing::Values(0, 4, 100, 1000, 10000),
         ::testing::Values(false, true)));
 
 TEST_F(AllgatherNewTest, TestTimeout) {

gloo/test/allgatherv_test.cc

@@ -86,7 +86,7 @@ INSTANTIATE_TEST_CASE_P(
     ::testing::Combine(
         ::testing::ValuesIn(kTransportsForFunctionAlgorithms),
         ::testing::Values(1, 2, 4, 7),
-        ::testing::Values(1, 10, 100, 1000),
+        ::testing::Values(0, 1, 10, 100, 1000),
         ::testing::Values(false, true)));
 
 TEST_F(AllgathervTest, TestTimeout) {