From 4f2b79f1e0c0e4dd637a275385702e0c69a7f7ec Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 20 Aug 2020 18:05:15 +0200 Subject: [PATCH 1/4] clean netcon / netpar --- coreneuron/network/netcon.hpp | 4 +-- coreneuron/network/netpar.cpp | 67 ++++++++++++----------------------- 2 files changed, 24 insertions(+), 47 deletions(-) diff --git a/coreneuron/network/netcon.hpp b/coreneuron/network/netcon.hpp index 543d340ad..687832300 100644 --- a/coreneuron/network/netcon.hpp +++ b/coreneuron/network/netcon.hpp @@ -174,10 +174,10 @@ class NetParEvent : public DiscreteEvent { double wx_, ws_; // exchange time and "spikes to Presyn" time NetParEvent(); - virtual ~NetParEvent(); + virtual ~NetParEvent() = default; virtual void send(double, NetCvode*, NrnThread*); virtual void deliver(double, NetCvode*, NrnThread*); - virtual int type() { + virtual int type() const { return NetParEventType; } diff --git a/coreneuron/network/netpar.cpp b/coreneuron/network/netpar.cpp index 989a822e3..be7bd686d 100644 --- a/coreneuron/network/netpar.cpp +++ b/coreneuron/network/netpar.cpp @@ -95,13 +95,12 @@ static bool active_ = false; static double usable_mindelay_; static double mindelay_; // the one actually used. Some of our optional algorithms static double last_maxstep_arg_; -static NetParEvent* npe_; // nrn_nthread of them -static int n_npe_; // just to compare with nrn_nthread +static std::vector npe_; // nrn_nthread of them #if NRNMPI // for combination of threads and mpi. #if defined(_OPENMP) -static MUTDEC +static OMP_Mutex mut; #endif #endif @@ -124,13 +123,11 @@ static MUTDEC #endif } -NetParEvent::NetParEvent() { - wx_ = ws_ = 0.; - ithread_ = -1; -} - -NetParEvent::~NetParEvent() { -} +NetParEvent::NetParEvent() + : ithread_(-1) + , wx_(0.) + , ws_(0.) +{} void NetParEvent::send(double tt, NetCvode* nc, NrnThread* nt) { nc->event(tt + usable_mindelay_, this, nt); @@ -167,7 +164,7 @@ void nrn_outputevent(unsigned char localgid, double firetime) { if (!active_) { return; } - MUTLOCK + std::lock_guard lock(mut); nout_++; int i = idxout_; idxout_ += 2; @@ -179,14 +176,13 @@ void nrn_outputevent(unsigned char localgid, double firetime) { spfixout_[i] = localgid; // printf("%d idx=%d lgid=%d firetime=%g t_exchange_=%g [0]=%d [1]=%d\n", nrnmpi_myid, i, // (int)localgid, firetime, t_exchange_, (int)spfixout_[i-1], (int)spfixout_[i]); - MUTUNLOCK } void nrn2ncs_outputevent(int gid, double firetime) { if (!active_) { return; } - MUTLOCK + std::lock_guard lock(mut); if (use_compress_) { nout_++; int i = idxout_; @@ -232,31 +228,23 @@ void nrn2ncs_outputevent(int gid, double firetime) { } #endif } - MUTUNLOCK // printf("%d cell %d in slot %d fired at %g\n", nrnmpi_myid, gid, i, firetime); } #endif // NRNMPI -static int nrn_need_npe() { - bool b = false; - if (active_) { - b = true; - } - if (nrn_nthread > 1) { - b = true; - } - if (b) { +static bool nrn_need_npe() { + if (active_ || nrn_nthread > 1) { if (last_maxstep_arg_ == 0) { last_maxstep_arg_ = 100.; } + return true; } else { - if (npe_) { - delete[] npe_; - npe_ = nullptr; - n_npe_ = 0; + if (!npe_.empty()) { + npe_.clear(); + npe_.shrint_to_fit(); } + return false; } - return b ? 1 : 0; } #define TBUFSIZE 0 @@ -295,12 +283,12 @@ void nrn_spike_exchange_init() { } #endif - if (n_npe_ != nrn_nthread) { - if (npe_) { - delete[] npe_; + if (npe_.size() != nrn_nthread) { + if (!npe_.empty()) { + npe_.clear(); + npe_.shrint_to_fit(); } - npe_ = new NetParEvent[nrn_nthread]; - n_npe_ = nrn_nthread; + npe_.resize(nrn_nthread); } for (int i = 0; i < nrn_nthread; ++i) { npe_[i].ithread_ = i; @@ -324,17 +312,6 @@ void nrn_spike_exchange_init() { } nout_ = 0; nsend_ = nsendmax_ = nrecv_ = nrecv_useful_ = 0; - if (nrnmpi_numprocs > 0) { - if (nrn_nthread > 0) { -#if defined(_OPENMP) - if (!mut_) { - MUTCONSTRUCT(1) - } -#endif - } else { - MUTDESTRUCT - } - } #endif // NRNMPI // if (nrnmpi_myid == 0){printf("usable_mindelay_ = %g\n", usable_mindelay_);} } @@ -720,7 +697,7 @@ void BBS_netpar_solve(double tstop) { ncs2nrn_integrate(tstop * (1. + 1e-11)); nrn_spike_exchange(nrn_threads); nrn_timeout(0); - if (npe_) { + if (!npe_.empty()) { npe_[0].wx_ = npe_[0].ws_ = 0.; }; // printf("%d netpar_solve exit t=%g tstop=%g mindelay_=%g\n",nrnmpi_myid, t, tstop, mindelay_); From 3c67a5aa994d9307fcdb1ea76c01fa39f160c5fe Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 20 Aug 2020 18:26:52 +0200 Subject: [PATCH 2/4] No more stats --- coreneuron/network/netpar.cpp | 101 ---------------------------------- 1 file changed, 101 deletions(-) diff --git a/coreneuron/network/netpar.cpp b/coreneuron/network/netpar.cpp index be7bd686d..e37fe059b 100644 --- a/coreneuron/network/netpar.cpp +++ b/coreneuron/network/netpar.cpp @@ -73,13 +73,6 @@ bool nrn_use_localgid_; void nrn_outputevent(unsigned char localgid, double firetime); std::vector > localmaps; -#define NRNSTAT 1 -static int nsend_, nsendmax_, nrecv_, nrecv_useful_; -#if NRNSTAT -/// Needs further allocation if desired to collect the histogram statistics -static IvocVect* max_histogram_; -#endif - static int ocapacity_; // for spikeout_ // require it to be smaller than min_interprocessor_delay. static double wt_; // wait time for nrnmpi_spike_exchange @@ -311,7 +304,6 @@ void nrn_spike_exchange_init() { #endif } nout_ = 0; - nsend_ = nsendmax_ = nrecv_ = nrecv_useful_ = 0; #endif // NRNMPI // if (nrnmpi_myid == 0){printf("usable_mindelay_ = %g\n", usable_mindelay_);} } @@ -335,12 +327,6 @@ void nrn_spike_exchange(NrnThread* nt) { nrnmpi_barrier(); #endif -#if NRNSTAT - nsend_ += nout_; - if (nsendmax_ < nout_) { - nsendmax_ = nout_; - } -#endif #if nrn_spikebuf_size > 0 spbufout_->nspike = nout_; #endif @@ -361,35 +347,8 @@ void nrn_spike_exchange(NrnThread* nt) { //} nout_ = 0; if (n == 0) { -#if NRNSTAT - if (max_histogram_) { - vector_vec(max_histogram_)[0] += 1.; - } -#endif return; } -#if NRNSTAT - nrecv_ += n; - if (max_histogram_) { - int mx = 0; - if (n > 0) { - for (int i = nrnmpi_numprocs - 1; i >= 0; --i) { -#if nrn_spikebuf_size == 0 - if (mx < nin_[i]) { - mx = nin_[i]; - } -#else - if (mx < spbufin_[i].nspike) { - mx = spbufin_[i].nspike; - } -#endif - } - } - int ms = vector_capacity(max_histogram_) - 1; - mx = (mx < ms) ? mx : ms; - vector_vec(max_histogram_)[mx] += 1.; - } -#endif // NRNSTAT #if nrn_spikebuf_size > 0 for (int i = 0; i < nrnmpi_numprocs; ++i) { int nn = spbufin_[i].nspike; @@ -401,9 +360,6 @@ void nrn_spike_exchange(NrnThread* nt) { if (gid2in_it != gid2in.end()) { InputPreSyn* ps = gid2in_it->second; ps->send(spbufin_[i].spiketime[j], net_cvode_instance, nt); -#if NRNSTAT - ++nrecv_useful_; -#endif } } } @@ -414,9 +370,6 @@ void nrn_spike_exchange(NrnThread* nt) { if (gid2in_it != gid2in.end()) { InputPreSyn* ps = gid2in_it->second; ps->send(spikein_[i].spiketime, net_cvode_instance, nt); -#if NRNSTAT - ++nrecv_useful_; -#endif } } wt1_ = nrn_wtime() - wt; @@ -430,12 +383,6 @@ void nrn_spike_exchange_compressed(NrnThread* nt) { nrnmpi_barrier(); #endif -#if NRNSTAT - nsend_ += nout_; - if (nsendmax_ < nout_) { - nsendmax_ = nout_; - } -#endif assert(nout_ < 0x10000); spfixout_[1] = (unsigned char)(nout_ & 0xff); spfixout_[0] = (unsigned char)(nout_ >> 8); @@ -455,30 +402,9 @@ void nrn_spike_exchange_compressed(NrnThread* nt) { nout_ = 0; idxout_ = 2; if (n == 0) { -#if NRNSTAT - if (max_histogram_) { - vector_vec(max_histogram_)[0] += 1.; - } -#endif t_exchange_ = nrn_threads->_t; return; } -#if NRNSTAT - nrecv_ += n; - if (max_histogram_) { - int mx = 0; - if (n > 0) { - for (int i = nrnmpi_numprocs - 1; i >= 0; --i) { - if (mx < nin_[i]) { - mx = nin_[i]; - } - } - } - int ms = vector_capacity(max_histogram_) - 1; - mx = (mx < ms) ? mx : ms; - vector_vec(max_histogram_)[mx] += 1.; - } -#endif // NRNSTAT if (nrn_use_localgid_) { int idxov = 0; for (int i = 0; i < nrnmpi_numprocs; ++i) { @@ -507,9 +433,6 @@ void nrn_spike_exchange_compressed(NrnThread* nt) { if (gid2in_it != gps.end()) { InputPreSyn* ps = gid2in_it->second; ps->send(firetime + 1e-10, net_cvode_instance, nt); -#if NRNSTAT - ++nrecv_useful_; -#endif } } for (; j < nn; ++j) { @@ -520,9 +443,6 @@ void nrn_spike_exchange_compressed(NrnThread* nt) { if (gid2in_it != gps.end()) { InputPreSyn* ps = gid2in_it->second; ps->send(firetime + 1e-10, net_cvode_instance, nt); -#if NRNSTAT - ++nrecv_useful_; -#endif } } } @@ -543,9 +463,6 @@ void nrn_spike_exchange_compressed(NrnThread* nt) { if (gid2in_it != gid2in.end()) { InputPreSyn* ps = gid2in_it->second; ps->send(firetime + 1e-10, net_cvode_instance, nt); -#if NRNSTAT - ++nrecv_useful_; -#endif } } } @@ -559,9 +476,6 @@ void nrn_spike_exchange_compressed(NrnThread* nt) { if (gid2in_it != gid2in.end()) { InputPreSyn* ps = gid2in_it->second; ps->send(firetime + 1e-10, net_cvode_instance, nt); -#if NRNSTAT - ++nrecv_useful_; -#endif } } } @@ -653,9 +567,6 @@ void nrn_fake_fire(int gid, double spiketime, int fake_out) { assert(psi); // printf("nrn_fake_fire %d %g\n", gid, spiketime); psi->send(spiketime, net_cvode_instance, nrn_threads); -#if NRNSTAT - ++nrecv_useful_; -#endif } else if (fake_out) { std::map::iterator gid2out_it; gid2out_it = gid2out.find(gid); @@ -664,9 +575,6 @@ void nrn_fake_fire(int gid, double spiketime, int fake_out) { assert(ps); // printf("nrn_fake_fire fake_out %d %g\n", gid, spiketime); ps->send(spiketime, net_cvode_instance, nrn_threads); -#if NRNSTAT - ++nrecv_useful_; -#endif } } } @@ -778,15 +686,6 @@ double set_mindelay(double maxdelay) { return mindelay_; } -void BBS_netpar_spanning_statistics(int* nsend, int* nsendmax, int* nrecv, int* nrecv_useful) { -#if NRNMPI - *nsend = nsend_; - *nsendmax = nsendmax_; - *nrecv = nrecv_; - *nrecv_useful = nrecv_useful_; -#endif -} - /* 08-Nov-2010 The workhorse for spike exchange on up to 10K machines is MPI_Allgather but as the number of machines becomes far greater than the fanout per From fa46163996fa60e5ba92d80fceb015b6dfa89c00 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 20 Aug 2020 18:50:40 +0200 Subject: [PATCH 3/4] Clean OMPMutex --- coreneuron/io/nrn_setup.cpp | 21 +++------------------ coreneuron/io/output_spikes.cpp | 14 ++++---------- coreneuron/io/phase1.cpp | 6 ------ coreneuron/io/phase1.hpp | 4 ---- coreneuron/network/netpar.cpp | 9 ++++----- coreneuron/utils/nrnmutdec.h | 26 ++++++++++++++++++++++++++ 6 files changed, 37 insertions(+), 43 deletions(-) diff --git a/coreneuron/io/nrn_setup.cpp b/coreneuron/io/nrn_setup.cpp index 2cd025a23..28fa1de07 100644 --- a/coreneuron/io/nrn_setup.cpp +++ b/coreneuron/io/nrn_setup.cpp @@ -164,9 +164,7 @@ void (*nrn2core_all_weights_return_)(std::vector& weights); namespace coreneuron { extern corenrn_parameters corenrn_param; -#ifdef _OPENMP static OMP_Mutex mut; -#endif static size_t model_size(void); @@ -479,13 +477,7 @@ void nrn_setup(const char* filesdat, Phase1 p1; p1.read_direct(n->id); NrnThread& nt = *n; - { -#ifdef _OPENMP - p1.populate(nt, mut); -#else - p1.populate(nt); -#endif - } + p1.populate(nt, mut); }); } @@ -538,9 +530,7 @@ void setup_ThreadData(NrnThread& nt) { ml->_thread = (ThreadDatum*)ecalloc_align(mf.thread_size_, sizeof(ThreadDatum)); if (mf.thread_mem_init_) { { -#ifdef _OPENMP const std::lock_guard lock(mut); -#endif (*mf.thread_mem_init_)(ml->_thread); } } @@ -860,13 +850,8 @@ void read_phase1(NrnThread& nt, UserParams& userParams) { Phase1 p1; p1.read_file(userParams.file_reader[nt.id]); - { // Protect gid2in, gid2out and neg_gid2out -#ifdef _OPENMP - p1.populate(nt, mut); -#else - p1.populate(nt); -#endif - } + // Protect gid2in, gid2out and neg_gid2out + p1.populate(nt, mut); } void read_phase2(NrnThread& nt, UserParams& userParams) { diff --git a/coreneuron/io/output_spikes.cpp b/coreneuron/io/output_spikes.cpp index 1c9839989..116855015 100644 --- a/coreneuron/io/output_spikes.cpp +++ b/coreneuron/io/output_spikes.cpp @@ -65,29 +65,23 @@ namespace coreneuron { std::vector spikevec_time; std::vector spikevec_gid; -#ifdef _OPENMP -static MUTDEC -#endif +static OMP_Mutex mut; - void - mk_spikevec_buffer(int sz) { +void mk_spikevec_buffer(int sz) { try { spikevec_time.reserve(sz); spikevec_gid.reserve(sz); } catch (const std::length_error& le) { std::cerr << "Lenght error" << le.what() << std::endl; } - if (!MUTCONSTRUCTED) { - MUTCONSTRUCT(1); - } } void spikevec_lock() { - MUTLOCK + mut.lock(); } void spikevec_unlock() { - MUTUNLOCK + mut.unlock(); } void local_spikevec_sort(std::vector& isvect, diff --git a/coreneuron/io/phase1.cpp b/coreneuron/io/phase1.cpp index e99738835..dd96bc4e1 100644 --- a/coreneuron/io/phase1.cpp +++ b/coreneuron/io/phase1.cpp @@ -43,11 +43,7 @@ void Phase1::read_direct(int thread_id) { delete[] netcon_srcgid; } -#ifdef _OPENMP void Phase1::populate(NrnThread& nt, OMP_Mutex& mut) { -#else -void Phase1::populate(NrnThread& nt) { -#endif nt.n_presyn = this->output_gids.size(); nt.n_netcon = this->netcon_srcgids.size(); @@ -68,9 +64,7 @@ void Phase1::populate(NrnThread& nt) { } { -#ifdef _OPENMP const std::lock_guard lock(mut); -#endif // Note that the negative (type, index) // coded information goes into the neg_gid2out[tid] hash table. // See netpar.cpp for the netpar_tid_... function implementations. diff --git a/coreneuron/io/phase1.hpp b/coreneuron/io/phase1.hpp index ef0d8d798..7d5ec5a05 100644 --- a/coreneuron/io/phase1.hpp +++ b/coreneuron/io/phase1.hpp @@ -13,11 +13,7 @@ class Phase1 { public: void read_file(FileHandler& F); void read_direct(int thread_id); -#ifdef _OPENMP void populate(NrnThread& nt, OMP_Mutex& mut); -#else - void populate(NrnThread& nt); -#endif private: std::vector output_gids; diff --git a/coreneuron/network/netpar.cpp b/coreneuron/network/netpar.cpp index e37fe059b..7c815690e 100644 --- a/coreneuron/network/netpar.cpp +++ b/coreneuron/network/netpar.cpp @@ -28,8 +28,9 @@ THE POSSIBILITY OF SUCH DAMAGE. #include #include -#include #include +#include +#include #include "coreneuron/nrnconf.h" #include "coreneuron/apps/corenrn_parameters.hpp" @@ -92,9 +93,7 @@ static std::vector npe_; // nrn_nthread of them #if NRNMPI // for combination of threads and mpi. -#if defined(_OPENMP) static OMP_Mutex mut; -#endif #endif /// Allocate space for spikes: 200 structs of {int gid; double time} @@ -234,7 +233,7 @@ static bool nrn_need_npe() { } else { if (!npe_.empty()) { npe_.clear(); - npe_.shrint_to_fit(); + npe_.shrink_to_fit(); } return false; } @@ -279,7 +278,7 @@ void nrn_spike_exchange_init() { if (npe_.size() != nrn_nthread) { if (!npe_.empty()) { npe_.clear(); - npe_.shrint_to_fit(); + npe_.shrink_to_fit(); } npe_.resize(nrn_nthread); } diff --git a/coreneuron/utils/nrnmutdec.h b/coreneuron/utils/nrnmutdec.h index a6da91175..fa31d3fa4 100644 --- a/coreneuron/utils/nrnmutdec.h +++ b/coreneuron/utils/nrnmutdec.h @@ -132,6 +132,32 @@ class OMP_Mutex { #define MUTDESTRUCT /**/ #define MUTLOCK /**/ #define MUTUNLOCK /**/ + +// This class respects the requirement *Mutex* +class OMP_Mutex { + public: + // Default constructible + OMP_Mutex() = default; + + // Destructible + ~OMP_Mutex() = default; + + // Not copyable + OMP_Mutex(const OMP_Mutex&) = delete; + OMP_Mutex& operator=(const OMP_Mutex&) = delete; + + // Not movable + OMP_Mutex(const OMP_Mutex&&) = delete; + OMP_Mutex& operator=(const OMP_Mutex&&) = delete; + + // Basic Lockable + void lock() {} + + void unlock() {} + + // Lockable + bool try_lock() {} +}; #endif #endif From 2a83c85fc74f5f907c34688208d00da9393f7372 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 20 Aug 2020 20:00:55 +0200 Subject: [PATCH 4/4] Put override on overrided methods --- coreneuron/network/netcon.hpp | 38 +++++++++++++++++------------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/coreneuron/network/netcon.hpp b/coreneuron/network/netcon.hpp index 687832300..a21047c19 100644 --- a/coreneuron/network/netcon.hpp +++ b/coreneuron/network/netcon.hpp @@ -57,7 +57,7 @@ class DiscreteEvent { virtual ~DiscreteEvent(); virtual void send(double deliverytime, NetCvode*, NrnThread*); virtual void deliver(double t, NetCvode*, NrnThread*); - virtual int type() { + virtual int type() const { return DiscreteEventType; } virtual bool require_checkpoint() { @@ -81,12 +81,12 @@ class NetCon : public DiscreteEvent { NetCon(); virtual ~NetCon(); - virtual void send(double sendtime, NetCvode*, NrnThread*); - virtual void deliver(double, NetCvode* ns, NrnThread*); - virtual int type() { + virtual void send(double sendtime, NetCvode*, NrnThread*) override; + virtual void deliver(double, NetCvode* ns, NrnThread*) override; + virtual int type() const override { return NetConType; } - virtual void pr(const char*, double t, NetCvode*); + virtual void pr(const char*, double t, NetCvode*) override; }; class SelfEvent : public DiscreteEvent { @@ -98,12 +98,12 @@ class SelfEvent : public DiscreteEvent { SelfEvent(); virtual ~SelfEvent(); - virtual void deliver(double, NetCvode*, NrnThread*); - virtual int type() { + virtual void deliver(double, NetCvode*, NrnThread*) override; + virtual int type() const override { return SelfEventType; } - virtual void pr(const char*, double t, NetCvode*); + virtual void pr(const char*, double t, NetCvode*) override; private: void call_net_receive(NetCvode*); @@ -138,13 +138,13 @@ class PreSyn : public ConditionEvent { PreSyn(); virtual ~PreSyn(); - virtual void send(double sendtime, NetCvode*, NrnThread*); - virtual void deliver(double, NetCvode*, NrnThread*); - virtual int type() { + virtual void send(double sendtime, NetCvode*, NrnThread*) override; + virtual void deliver(double, NetCvode*, NrnThread*) override; + virtual int type() const override { return PreSynType; } - virtual double value(NrnThread*); + virtual double value(NrnThread*) override; void record(double t); #if NRN_MULTISEND int multisend_index_; @@ -158,9 +158,9 @@ class InputPreSyn : public DiscreteEvent { InputPreSyn(); virtual ~InputPreSyn(); - virtual void send(double sendtime, NetCvode*, NrnThread*); - virtual void deliver(double, NetCvode*, NrnThread*); - virtual int type() { + virtual void send(double sendtime, NetCvode*, NrnThread*) override; + virtual void deliver(double, NetCvode*, NrnThread*) override; + virtual int type() const override { return InputPreSynType; } #if NRN_MULTISEND @@ -175,13 +175,13 @@ class NetParEvent : public DiscreteEvent { NetParEvent(); virtual ~NetParEvent() = default; - virtual void send(double, NetCvode*, NrnThread*); - virtual void deliver(double, NetCvode*, NrnThread*); - virtual int type() const { + virtual void send(double, NetCvode*, NrnThread*) override; + virtual void deliver(double, NetCvode*, NrnThread*) override; + virtual int type() const override { return NetParEventType; } - virtual void pr(const char*, double t, NetCvode*); + virtual void pr(const char*, double t, NetCvode*) override; }; } // namespace coreneuron #endif