Address comments

Author: leezu

scripts/word_embeddings/tools/cooccur.cc

@@ -59,15 +59,16 @@ std::ostream &operator<<(std::ostream &in,
   return in << static_cast<int>(context_weight);
 }
 
+// Arguments specified via command line options. See ParseArgs for documentation.
 struct Arguments {
   unsigned int num_threads = 1;
-  unsigned int windowSize = 5;
-  bool noSymmetric = false;
+  unsigned int window_size = 5;
+  bool no_symmetric = false;
   bool subsample = false;
   ContextWeight context_weight;
 };
 
-auto parseArgs(int argc, char **argv) {
+auto ParseArgs(int argc, char **argv) {
   // Performance optimizations for writing to stdout
   std::ios::sync_with_stdio(false);
 
@@ -79,9 +80,9 @@ auto parseArgs(int argc, char **argv) {
   app.add_option("-o,--output", output,
                  "Output file name. Co-occurence matrix is saved as "
                  "scipy.sparse compatible CSR matrix in a numpy .npz archive");
-  app.add_option("-w,--window-size", args.windowSize,
+  app.add_option("-w,--window-size", args.window_size,
                  "Window size in which to count co-occurences.");
-  app.add_flag("--no-symmetric", args.noSymmetric,
+  app.add_flag("--no-symmetric", args.no_symmetric,
                "If not specified, a symmetric context window is used.");
   app.add_flag("--subsample", args.subsample,
                "Apply subsampling during co-occurence matrix construction as "
@@ -178,7 +179,7 @@ class CircularBuffer : public ranges::view_facade<CircularBuffer<T>> {
 };
 
 // * Input
-auto readVocab() {
+auto ReadVocab() {
   std::string word;
   std::string count;
   int rank{0};
@@ -194,11 +195,11 @@ auto readVocab() {
 std::mutex paths_m;
 std::mutex matrices_m;
 
-void readMatrix(std::queue<fs::path> &paths, queue<Matrix> &matrices,
+void ReadMatrix(std::queue<fs::path> &paths, queue<Matrix> &matrices,
                 const Vocab &vocab, const Arguments &args, uint32_t seed) {
   assert(seed > 0);
   std::string line;
-  CircularBuffer<uint32_t> history(args.windowSize);
+  CircularBuffer<uint32_t> history(args.window_size);
   std::unique_ptr<Matrix> m = std::make_unique<Matrix>();
 
   // Prepare subsampling
@@ -255,7 +256,7 @@ void readMatrix(std::queue<fs::path> &paths, queue<Matrix> &matrices,
             uint64_t key; // We merge 32 bit row and col indices to a single 64
                           // bit key
             // For symmetric contexts, only store one direction.
-            if (!args.noSymmetric) {
+            if (!args.no_symmetric) {
               if (word_rank <= context_word_rank) {
                 key = (uint64_t)word_rank << 32 | context_word_rank;
               } else {
@@ -268,7 +269,7 @@ void readMatrix(std::queue<fs::path> &paths, queue<Matrix> &matrices,
             if (args.context_weight == ContextWeight::Harmonic) {
               (*m)[key] += 1.0f / static_cast<count_type>(distance);
             } else if (args.context_weight == ContextWeight::DistanceOverSize) {
-              (*m)[key] += (args.windowSize - distance - 1) / args.windowSize;
+              (*m)[key] += (args.window_size - distance - 1) / args.window_size;
             } else {
               (*m)[key]++;
             }
@@ -286,7 +287,7 @@ void readMatrix(std::queue<fs::path> &paths, queue<Matrix> &matrices,
   }
 }
 
-std::unique_ptr<Matrix> combineMatrices(queue<Matrix> &matrices,
+std::unique_ptr<Matrix> CombineMatrices(queue<Matrix> &matrices,
                                         int num_threads) {
   std::unique_ptr<Matrix> m1 = matrices.pop();
   for (int i = 1; i < num_threads; i++) {
@@ -305,24 +306,24 @@ std::unique_ptr<Matrix> combineMatrices(queue<Matrix> &matrices,
   return m1;
 }
 
-auto computeCooccurrenceMatrix(Vocab &vocab, std::queue<fs::path> &paths,
+auto ComputeCooccurrenceMatrix(Vocab &vocab, std::queue<fs::path> &paths,
                                const Arguments &args) {
   std::vector<std::thread> threads;
   queue<Matrix> matrices;
   for (unsigned int i = 0; i < args.num_threads; i++) {
     threads.push_back(std::thread([&paths, &matrices, &vocab, &args, i]() {
-      readMatrix(std::ref(paths), std::ref(matrices), std::ref(vocab),
+      ReadMatrix(std::ref(paths), std::ref(matrices), std::ref(vocab),
                  std::ref(args), i + 1);
     }));
   }
-  std::unique_ptr<Matrix> m = combineMatrices(matrices, args.num_threads);
+  std::unique_ptr<Matrix> m = CombineMatrices(matrices, args.num_threads);
   for (unsigned int i = 0; i < args.num_threads; i++) {
     threads[i].join();
   }
   return m;
 }
 
-auto toCOO(const Vocab &vocab, std::unique_ptr<Matrix> m) {
+auto ToCOO(const Vocab &vocab, std::unique_ptr<Matrix> m) {
   size_t num_tokens = vocab.size();
   size_t nnz = m->size();
   std::cout << "Got " << nnz
@@ -343,7 +344,7 @@ auto toCOO(const Vocab &vocab, std::unique_ptr<Matrix> m) {
 }
 
 // * Output
-void writeNumpy(const std::string output, const std::vector<uint32_t> &row,
+void WriteNumpy(const std::string output, const std::vector<uint32_t> &row,
                 const std::vector<uint32_t> &col,
                 const std::vector<count_type> &data, const bool symmetric,
                 const uint32_t num_tokens) {
@@ -359,10 +360,10 @@ void writeNumpy(const std::string output, const std::vector<uint32_t> &row,
 
 // * Main
 int main(int argc, char **argv) {
-  auto [paths, output, args] = parseArgs(argc, argv);
-  auto vocab = readVocab();
-  auto cooccurenceMatrix = computeCooccurrenceMatrix(vocab, paths, args);
-  auto [row, col, data] = toCOO(vocab, std::move(cooccurenceMatrix));
-  writeNumpy(output, row, col, data, !args.noSymmetric, vocab.size());
+  auto [paths, output, args] = ParseArgs(argc, argv);
+  auto vocab = ReadVocab();
+  auto cooccurenceMatrix = ComputeCooccurrenceMatrix(vocab, paths, args);
+  auto [row, col, data] = ToCOO(vocab, std::move(cooccurenceMatrix));
+  WriteNumpy(output, row, col, data, !args.no_symmetric, vocab.size());
   return 0;
 }

scripts/word_embeddings/tools/utils.h

@@ -14,13 +14,17 @@ template <typename T> class queue {
   std::deque<std::unique_ptr<T>> d_queue;
 
 public:
+  // Add a value to queue in a thread-safe manner.
   void push(std::unique_ptr<T> value) {
     {
       std::unique_lock<std::mutex> lock(this->d_mutex);
       d_queue.push_front(std::move(value));
     }
     this->d_condition.notify_one();
   }
+
+  // Remove and return a value from the queue in a thread-safe manner (FIFO).
+  // Blocks if there is no value in the Queue.
   std::unique_ptr<T> pop() {
     std::unique_lock<std::mutex> lock(this->d_mutex);
     this->d_condition.wait(lock, [&] { return !this->d_queue.empty(); });

scripts/word_embeddings/tools/vocab_count.cc

@@ -44,7 +44,7 @@ using Vocab = spp::sparse_hash_map<std::string, uint32_t>;
 std::mutex paths_m;
 std::mutex vocabs_m;
 
-void readVocab(std::queue<fs::path> &paths, queue<Vocab> &vocabs) {
+void ReadVocab(std::queue<fs::path> &paths, queue<Vocab> &vocabs) {
   std::unique_ptr<Vocab> vocab = std::make_unique<Vocab>();
   std::string word;
   fs::path path;
@@ -73,9 +73,9 @@ void readVocab(std::queue<fs::path> &paths, queue<Vocab> &vocabs) {
   }
 }
 
-std::unique_ptr<Vocab> combineVocabs(queue<Vocab> &vocabs, int numThreads) {
+std::unique_ptr<Vocab> CombineVocabs(queue<Vocab> &vocabs, int num_threads) {
   std::unique_ptr<Vocab> vocab1 = vocabs.pop();
-  for (int i = 1; i < numThreads; i++) {
+  for (int i = 1; i < num_threads; i++) {
     std::unique_ptr<Vocab> vocab2 = vocabs.pop();
     if (vocab1->size() < vocab2->size()) {
       for (const auto &e : *vocab1) {
@@ -102,9 +102,9 @@ int main(int argc, char **argv) {
   app.add_option("-c,--minCount", minCount,
                  "Minimum number of occurences required for a word to be "
                  "included in the vocabulary.");
-  unsigned int numThreads = 1;
+  unsigned int num_threads = 1;
   app.add_option(
-         "-j,--numThreads", numThreads,
+         "-j,--num_threads", num_threads,
          "Number of threads to use. Each thread constructs an "
          "independent vocabulary which are finally merged. Only appropriate "
          "when multiple, sufficiently large input files are specified.")
@@ -117,26 +117,26 @@ int main(int argc, char **argv) {
   }
   std::vector<std::thread> threads;
   queue<Vocab> vocabs;
-  for (unsigned int i = 0; i < numThreads; i++) {
+  for (unsigned int i = 0; i < num_threads; i++) {
     threads.push_back(std::thread(
-        [&paths, &vocabs]() { readVocab(std::ref(paths), std::ref(vocabs)); }));
+        [&paths, &vocabs]() { ReadVocab(std::ref(paths), std::ref(vocabs)); }));
   }
-  std::unique_ptr<Vocab> vocab = combineVocabs(vocabs, numThreads);
-  for (unsigned int i = 0; i < numThreads; i++) {
+  std::unique_ptr<Vocab> vocab = CombineVocabs(vocabs, num_threads);
+  for (unsigned int i = 0; i < num_threads; i++) {
     threads[i].join();
   }
 
   // Sort
   typedef std::function<bool(std::pair<std::string, int>,
                              std::pair<std::string, int>)>
       Comparator;
-  Comparator compFunctor = [](std::pair<std::string, int> elem1,
+  Comparator CompFunctor = [](std::pair<std::string, int> elem1,
                               std::pair<std::string, int> elem2) {
     return (elem1.second > elem2.second) ||
            (elem1.second == elem2.second && elem1.first < elem2.first);
   };
   std::set<std::pair<std::string, uint32_t>, Comparator> sorted_vocab(
-      vocab->begin(), vocab->end(), compFunctor);
+      vocab->begin(), vocab->end(), CompFunctor);
   vocab.reset(); // Release ownership
 
   // Output

scripts/word_embeddings/train_glove.py

@@ -237,12 +237,18 @@ def hybrid_forward(self, F, row, col, counts):
 
         Parameters
         ----------
-        row : mx.nd.NDArray
-            Array of token indices for source words
-        row : mx.nd.NDArray
-            Array of token indices for context words
-        counts : mx.nd.NDArray
-            Their co-occurrence counts.
+        row : mxnet.nd.NDArray or mxnet.sym.Symbol
+            Array of token indices for source words. Shape (batch_size, ).
+        row : mxnet.nd.NDArray or mxnet.sym.Symbol
+            Array of token indices for context words. Shape (batch_size, ).
+        counts : mxnet.nd.NDArray or mxnet.sym.Symbol
+            Their co-occurrence counts. Shape (batch_size, ).
+
+        Returns
+        -------
+        mxnet.nd.NDArray or mxnet.sym.Symbol
+            Loss. Shape (batch_size, ).
+
         """
 
         emb_in = self.source_embedding(row)