From 25228edca0b3fb54325c0e105618866265e7453e Mon Sep 17 00:00:00 2001
From: Sam Yates <halfflat@gmail.com>
Date: Thu, 4 Aug 2016 14:39:48 +0200
Subject: [PATCH] Address PR#64 review comments.

---
 miniapp/io.cpp                     |  2 +-
 miniapp/miniapp.cpp                |  5 ++++-
 miniapp/miniapp_recipes.cpp        | 10 +++++-----
 miniapp/trace_sampler.hpp          |  1 -
 src/communication/communicator.hpp | 10 ----------
 src/recipe.hpp                     |  8 +++++++-
 src/tree.hpp                       |  8 ++++----
 7 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/miniapp/io.cpp b/miniapp/io.cpp
index 93a8d5e0..6664cf47 100644
--- a/miniapp/io.cpp
+++ b/miniapp/io.cpp
@@ -85,7 +85,7 @@ cl_options read_options(int argc, char** argv) {
             }
         }
         else {
-            throw usage_error("unable to open model paramter file "+options.ifname);
+            throw usage_error("unable to open model parameter file "+options.ifname);
         }
     }
 
diff --git a/miniapp/miniapp.cpp b/miniapp/miniapp.cpp
index 898e478c..9b26ff86 100644
--- a/miniapp/miniapp.cpp
+++ b/miniapp/miniapp.cpp
@@ -98,7 +98,10 @@ int main(int argc, char** argv) {
 }
 
 std::pair<cell_gid_type, cell_gid_type> distribute_cells(cell_size_type num_cells) {
-    // crude load balancing:
+    // Crude load balancing:
+    // divide [0, num_cells) into num_domains non-overlapping, contiguous blocks
+    // of size as close to equal as possible.
+
     auto num_domains = communication::global_policy::size();
     auto domain_id = communication::global_policy::id();
 
diff --git a/miniapp/miniapp_recipes.cpp b/miniapp/miniapp_recipes.cpp
index 0c8b7a2d..22c59bd8 100644
--- a/miniapp/miniapp_recipes.cpp
+++ b/miniapp/miniapp_recipes.cpp
@@ -43,7 +43,7 @@ cell make_basic_cell(
 
     auto distribution = std::uniform_real_distribution<float>(0.f, 1.0f);
     // distribute the synapses at random locations the terminal dendrites in a
-    // round robin manner
+    // round robin manner; the terminal dendrites in this cell have indices 2 and 3.
     nest::mc::parameter_list syn_default(syn_type);
     for (unsigned i=0; i<num_synapses; ++i) {
         cell.add_synapse({2+(i%2), distribution(rng)}, syn_default);
@@ -64,7 +64,7 @@ public:
     cell_size_type num_cells() const override { return ncell_; }
 
     cell get_cell(cell_gid_type i) const override {
-        auto gen = std::mt19937(i); // replace this with hashing generator...
+        auto gen = std::mt19937(i); // TODO: replace this with hashing generator...
 
         auto cc = get_cell_count_info(i);
         auto cell = make_basic_cell(param_.num_compartments, cc.num_targets,
@@ -132,7 +132,7 @@ public:
 
     std::vector<cell_connection> connections_on(cell_gid_type i) const override {
         std::vector<cell_connection> conns;
-        auto gen = std::mt19937(i); // replace this with hashing generator...
+        auto gen = std::mt19937(i); // TODO: replace this with hashing generator...
 
         cell_gid_type prev = i==0? ncell_-1: i-1;
         for (unsigned t=0; t<param_.num_synapses; ++t) {
@@ -164,7 +164,7 @@ public:
 
     std::vector<cell_connection> connections_on(cell_gid_type i) const override {
         std::vector<cell_connection> conns;
-        auto conn_param_gen = std::mt19937(i); // replace this with hashing generator...
+        auto conn_param_gen = std::mt19937(i); // TODO: replace this with hashing generator...
         auto source_gen = std::mt19937(i*123+457); // ditto
 
         std::uniform_int_distribution<cell_gid_type> source_distribution(0, ncell_-2);
@@ -206,7 +206,7 @@ public:
 
     std::vector<cell_connection> connections_on(cell_gid_type i) const override {
         std::vector<cell_connection> conns;
-        auto conn_param_gen = std::mt19937(i); // replace this with hashing generator...
+        auto conn_param_gen = std::mt19937(i); // TODO: replace this with hashing generator...
 
         for (unsigned t=0; t<param_.num_synapses; ++t) {
             cell_gid_type source = t>=i? t+1: t;
diff --git a/miniapp/trace_sampler.hpp b/miniapp/trace_sampler.hpp
index 1e20bea2..26db0a49 100644
--- a/miniapp/trace_sampler.hpp
+++ b/miniapp/trace_sampler.hpp
@@ -12,7 +12,6 @@
 namespace nest {
 namespace mc {
 
-// move sampler code to another source file...
 template <typename Time=float, typename Value=double>
 struct sample_trace {
     using time_type = Time;
diff --git a/src/communication/communicator.hpp b/src/communication/communicator.hpp
index 9336daeb..871bf652 100644
--- a/src/communication/communicator.hpp
+++ b/src/communication/communicator.hpp
@@ -91,19 +91,14 @@ public:
     void exchange() {
         // global all-to-all to gather a local copy of the global spike list
         // on each node
-        //profiler_.enter("global exchange");
         auto global_spikes = communication_policy_.gather_spikes(local_spikes());
         num_spikes_ += global_spikes.size();
         clear_thread_spike_buffers();
-        //profiler_.leave();
 
         for (auto& q : events_) {
             q.clear();
         }
 
-        //profiler_.enter("events");
-
-        //profiler_.enter("make events");
         // check all global spikes to see if they will generate local events
         for (auto spike : global_spikes) {
             // search for targets
@@ -118,11 +113,6 @@ public:
                 events_[gidx].push_back(it->make_event(spike));
             }
         }
-
-
-        //profiler_.leave(); // make events
-
-        //profiler_.leave(); // event generation
     }
 
     uint64_t num_spikes() const { return num_spikes_; }
diff --git a/src/recipe.hpp b/src/recipe.hpp
index fe299242..b451a1a1 100644
--- a/src/recipe.hpp
+++ b/src/recipe.hpp
@@ -27,6 +27,12 @@ public:
 
 using cell_connection_endpoint = cell_member_type;
 
+// Note: `cell_connection` and `connection` have essentially the same data
+// and represent the same thing conceptually. `cell_connection` objects
+// are notionally described in terms of external cell identifiers instead
+// of internal gids, but we are not making the distinction between the
+// two in the current code. These two types could well be merged.
+
 struct cell_connection {
     cell_connection_endpoint source;
     cell_connection_endpoint dest;
@@ -39,7 +45,7 @@ class recipe {
 public:
     virtual cell_size_type num_cells() const =0;
 
-    virtual cell get_cell(cell_gid_type) const =0; 
+    virtual cell get_cell(cell_gid_type) const =0;
     virtual cell_count_info get_cell_count_info(cell_gid_type) const =0;
     virtual std::vector<cell_connection> connections_on(cell_gid_type) const =0;
 };
diff --git a/src/tree.hpp b/src/tree.hpp
index 6d5eeae7..cd595374 100644
--- a/src/tree.hpp
+++ b/src/tree.hpp
@@ -1,4 +1,4 @@
- #pragma once
+#pragma once
 
 #include <algorithm>
 #include <cassert>
@@ -12,13 +12,13 @@
 namespace nest {
 namespace mc {
 
-template <typename IntT, typename SizeT = std::size_t>
+template <typename Int, typename Size = std::size_t>
 class tree {
     using range = memory::Range;
 
 public:
-    using int_type = IntT;
-    using size_type = SizeT;
+    using int_type = Int;
+    using size_type = Size;
 
     using index_type = memory::HostVector<int_type>;
     using view_type  = typename index_type::view_type;
-- 
GitLab