review probe functionality from @halfflat

* change `probe_sort` enum to scoped enum
    - renamed to `probeKind`
    - refactored out of class
    - updated coding guidelines wiki with enum rules
* refactor `std::pair` into structs with meaningfull name types in `cell.hpp`
    - not just probes: stimulii and detectors too.
* add profiling region around sampling in `cell_group`
* change output data format for traces to json
* remove white space at end of lines (looking at you Sam)

commit.msg

+* change `probe_sort` enum to scoped enum
+    - renamed to `probeKind`
+    - refactored out of class
+    - updated coding guidelines wiki with enum rules
+* refactor `std::pair` into structs with meaningfull name types in `cell.hpp`
+    - not just probes: stimulii and detectors too.
+* add profiling region around sampling in `cell_group`
+* change output data format for traces to json
+* remove white space at end of lines (looking at you Sam)

miniapp/miniapp.cpp

@@ -100,7 +100,10 @@ struct model {
 
     // traces from probes
     struct trace_data {
-        using sample_type = std::pair<float,double>;
+        struct sample_type {
+            float time;
+            double value;
+        };
         std::string name;
         index_type id;
         std::vector<sample_type> samples;
@@ -120,7 +123,8 @@ struct model {
         float dt_ = 0;
 
         simple_sampler_functor(std::vector<trace_data> &traces, size_t index, float dt) :
-            traces_(traces), trace_index_(index), dt_(dt) {}
+            traces_(traces), trace_index_(index), dt_(dt)
+        {}
 
         optional<float> operator()(float t, double v) {
             traces_[trace_index_].samples.push_back({t,v});
@@ -128,8 +132,8 @@ struct model {
         }
     };
 
-    mc::sampler make_simple_sampler(index_type probe_gid, const std::string name,
-                                       index_type id, float dt)
+    mc::sampler make_simple_sampler(
+        index_type probe_gid, const std::string name, index_type id, float dt)
     {
         traces.push_back(trace_data{name, id});
         return {probe_gid, simple_sampler_functor(traces, traces.size()-1, dt)};
@@ -143,15 +147,17 @@ struct model {
     void dump_traces() {
         // do not call during simulation: thread-unsafe access to traces.
         for (const auto& trace: traces) {
-            std::stringstream path;
-            path << "trace_" << trace.id << "_" << trace.name << ".dat";
+            auto path = "trace_" + std::to_string(trace.id)
+                      + "_" + trace.name + ".json";
 
-            std::ofstream file(path.str());
-            file << "time\t" << trace.name << "\n";
+            nlohmann::json json;
+            json["name"] = trace.name;
             for (const auto& sample: trace.samples) {
-                file << sample.first << "\t" << sample.second << "\n";
+                json["time"].push_back(sample.time);
+                json["value"].push_back(sample.value);
             }
-            file.close();
+            std::ofstream file(path);
+            file << std::setw(1) << json << std::endl;
         }
     }
 };
@@ -403,8 +409,8 @@ mc::cell make_cell(int compartments_per_segment, int num_synapses) {
     }
 
     // add probes: 
-    auto probe_soma = cell.add_probe({0, 0}, mc::cell::membrane_voltage);
-    auto probe_dendrite = cell.add_probe({1, 0.5}, mc::cell::membrane_voltage);
+    auto probe_soma = cell.add_probe({0, 0}, mc::probeKind::membrane_voltage);
+    auto probe_dendrite = cell.add_probe({1, 0.5}, mc::probeKind::membrane_voltage);
 
     EXPECTS(probe_soma==0);
     EXPECTS(probe_dendrite==1);

src/cell.hpp

@@ -38,6 +38,11 @@ int find_compartment_index(
     compartment_model const& graph
 );
 
+enum class probeKind {
+    membrane_voltage,
+    membrane_current
+};
+
 /// high-level abstract representation of a cell and its segments
 class cell {
 public:
@@ -46,8 +51,18 @@ public:
     using index_type = int;
     using value_type = double;
     using point_type = point<value_type>;
-
-    enum probe_sort { membrane_voltage, membrane_current };
+    struct probe_instance {
+        segment_location location;
+        probeKind kind;
+    };
+    struct stimulus_instance {
+        segment_location location;
+        i_clamp clamp;
+    };
+    struct detector_instance {
+        segment_location location;
+        double threshold;
+    };
 
     // constructor
     cell();
@@ -109,12 +124,12 @@ public:
     //////////////////
     void add_stimulus(segment_location loc, i_clamp stim);
 
-    std::vector<std::pair<segment_location, i_clamp>>&
+    std::vector<stimulus_instance>&
     stimulii() {
         return stimulii_;
     }
 
-    const std::vector<std::pair<segment_location, i_clamp>>&
+    const std::vector<stimulus_instance>&
     stimulii() const {
         return stimulii_;
     }
@@ -131,12 +146,12 @@ public:
     //////////////////
     void add_detector(segment_location loc, double threshold);
 
-    std::vector<std::pair<segment_location, double>>&
+    std::vector<detector_instance>&
     detectors() {
         return spike_detectors_;
     }
 
-    const std::vector<std::pair<segment_location, double>>&
+    const std::vector<detector_instance>&
     detectors() const {
         return spike_detectors_;
     }
@@ -144,12 +159,12 @@ public:
     //////////////////
     // probes
     //////////////////
-    index_type add_probe(segment_location loc, enum probe_sort sort) {
-        probes_.push_back({loc, sort});
+    index_type add_probe(segment_location loc, probeKind kind) {
+        probes_.push_back({loc, kind});
         return probes_.size()-1;
     }
 
-    const std::vector<std::pair<segment_location, enum probe_sort>>&
+    const std::vector<probe_instance>&
     probes() const { return probes_; }
 
 private:
@@ -161,16 +176,16 @@ private:
     std::vector<segment_ptr> segments_;
 
     // the stimulii
-    std::vector<std::pair<segment_location, i_clamp>> stimulii_;
+    std::vector<stimulus_instance> stimulii_;
 
     // the synapses
     std::vector<segment_location> synapses_;
 
     // the sensors
-    std::vector<std::pair<segment_location, double>> spike_detectors_;
+    std::vector<detector_instance> spike_detectors_;
 
     // the probes
-    std::vector<std::pair<segment_location, enum probe_sort>> probes_;
+    std::vector<probe_instance> probes_;
 };
 
 // Checks that two cells have the same

src/cell_group.hpp

@@ -24,7 +24,7 @@ struct sampler {
     index_type probe_gid;   // samplers are attached to probes
     std::function<util::optional<time_type>(time_type, value_type)> sample;
 };
-    
+
 template <typename Cell>
 class cell_group {
 public:
@@ -49,7 +49,7 @@ public:
 
         for (auto& d : c.detectors()) {
             spike_sources_.push_back( {
-                0u, spike_detector_type(cell_, d.first, d.second, 0.f)
+                0u, spike_detector_type(cell_, d.location, d.threshold, 0.f)
             });
         }
     }
@@ -76,23 +76,14 @@ public:
         return { first_probe_gid_, first_probe_gid_+cell_.num_probes() };
     }
 
-#ifdef SPLAT
-    void splat(std::string fname) {
-        char buffer[128];
-        std::ofstream fid(fname);
-        for (auto i=0u; i<tt.size(); ++i) {
-            sprintf(buffer, "%8.4f %16.8f %16.8f\n", tt[i], vs[i], vd[i]);
-            fid << buffer;
-        }
-    }
-#endif
-
     void advance(double tfinal, double dt) {
         while (cell_.time()<tfinal) {
             // take any pending samples
             float cell_time = cell_.time();
+
+                nest::mc::util::profiler_enter("sampling");
             while (auto m = sample_events_.pop_if_before(cell_time)) {
-                auto &sampler = samplers_[m->sampler_index];
+                auto& sampler = samplers_[m->sampler_index];
                 EXPECTS((bool)sampler.sample);
 
                 index_type probe_index = sampler.probe_gid-first_probe_gid_;
@@ -102,12 +93,8 @@ public:
                     sample_events_.push(*m);
                 }
             }
+                nest::mc::util::profiler_leave();
 
-#ifdef SPLAT
-            tt.push_back(cell_.time());
-            vs.push_back(cell_.voltage({0,0.0}));
-            vd.push_back(cell_.voltage({1,0.5}));
-#endif
             // look for events in the next time step
             auto tstep = std::min(tfinal, cell_.time()+dt);
             auto next = events_.pop_if_before(tstep);
@@ -164,20 +151,14 @@ public:
         spikes_.clear();
     }
 
-    void add_sampler(const sampler &s, float start_time = 0) {
-        unsigned sampler_index = samplers_.size();
+    void add_sampler(const sampler& s, float start_time = 0) {
+        auto sampler_index = uint32_t(samplers_.size());
         samplers_.push_back(s);
         sample_events_.push({sampler_index, start_time});
     }
-        
+
 private:
 
-#ifdef SPLAT
-    // REMOVE as soon as we have a better way to probe cell state
-    std::vector<float> tt;
-    std::vector<float> vs;
-    std::vector<float> vd;
-#endif
 
     /// the lowered cell state (e.g. FVM) of the cell
     cell_type cell_;

src/fvm_cell.hpp

@@ -382,8 +382,8 @@ fvm_cell<T, I>::fvm_cell(nest::mc::cell const& cell)
 
     // add the stimulii
     for(const auto& stim : cell.stimulii()) {
-        auto idx = find_compartment_index(stim.first, graph);
-        stimulii_.push_back( {idx, stim.second} );
+        auto idx = find_compartment_index(stim.location, graph);
+        stimulii_.push_back( {idx, stim.clamp} );
     }
 
     // add the synapses
@@ -406,16 +406,16 @@ fvm_cell<T, I>::fvm_cell(nest::mc::cell const& cell)
 
     // record probe locations by index into corresponding state vector
     for (auto probe : cell.probes()) {
-        uint32_t comp = find_compartment_index(probe.first, graph);
-        switch (probe.second) {
-        case nest::mc::cell::membrane_voltage:
-            probes_.push_back({&fvm_cell::voltage_, comp});
-            break;
-        case nest::mc::cell::membrane_current:
-            probes_.push_back({&fvm_cell::current_, comp});
-            break;
-        default:
-            throw std::logic_error("unrecognized probe sort");
+        uint32_t comp = find_compartment_index(probe.location, graph);
+        switch (probe.kind) {
+            case probeKind::membrane_voltage:
+                probes_.push_back({&fvm_cell::voltage_, comp});
+                break;
+            case probeKind::membrane_current:
+                probes_.push_back({&fvm_cell::current_, comp});
+                break;
+            default:
+                throw std::logic_error("unrecognized probeKind");
         }
     }
 }