From 4e04709fc49d4b8b994784daccdfb0d6745aa692 Mon Sep 17 00:00:00 2001 From: Pramod Kumbhar Date: Fri, 16 Jul 2021 21:37:24 +0200 Subject: [PATCH 1/3] Bug fix for the GPU execution of BBP model: issue with mechanism data offset * Analysis of the bug: - Blue Brain model (channel-benchmark) was showing diferent spikes after few timesteps. Spikes were shifted and initially appeared to be floating point discrepencies. - Closely looking into NET_RECEIVE method revealed that the mechanism data accessed in net_receieve function was wrong. Somehow accessing variables like NMDA_ratio was showing wrong values. The nrn_state() had correct values. - The net_receieve function was accessing ml->data inside OpenACC loop. Accessing ml->data was providing different pointer inside OpenACC loop. * Fix for the bug: - some mechanisms can have dparam semantics for diam data and it is added into nt->data (after matrix elements). - When calculating offert for mechanism data, this extra semantics data was not considered. Adjusting the offset fixes the issue. - Additionally, we do not need to rely on offset but we can use acc_deviceptr to find corresponding device pointer directly. This is more reliable than manually calculating offset. --- coreneuron/gpu/nrn_acc_manager.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/coreneuron/gpu/nrn_acc_manager.cpp b/coreneuron/gpu/nrn_acc_manager.cpp index 1ea94a394..32477dad0 100644 --- a/coreneuron/gpu/nrn_acc_manager.cpp +++ b/coreneuron/gpu/nrn_acc_manager.cpp @@ -139,6 +139,15 @@ void setup_nrnthreads_on_device(NrnThread* threads, int nthreads) { acc_memcpy_to_device(&(d_nt->_actual_diam), &(dptr), sizeof(double*)); } + size_t offset = 6 * ne; + + // see Phase2::populate(): some mechanisms can have diam semantics + // added after matrix data in nt._data. Take that into account while + // calculating the offset for start of mechanism data. + if (nt->_actual_diam) { + offset += ne; + } + int* d_v_parent_index = (int*) acc_copyin(nt->_v_parent_index, nt->end * sizeof(int)); acc_memcpy_to_device(&(d_nt->_v_parent_index), &(d_v_parent_index), sizeof(int*)); @@ -153,7 +162,6 @@ void setup_nrnthreads_on_device(NrnThread* threads, int nthreads) { NrnThreadMembList* d_last_tml; bool first_tml = true; - size_t offset = 6 * ne; for (auto tml = nt->tml; tml; tml = tml->next) { /*copy tml to device*/ From 2e168d5ed77f5b7629190856a32aac60b581fb23 Mon Sep 17 00:00:00 2001 From: Pramod Kumbhar Date: Fri, 16 Jul 2021 21:57:46 +0200 Subject: [PATCH 2/3] Rely on acc_deviceptr (reliable) than manually calculating offsets (fragile) --- coreneuron/gpu/nrn_acc_manager.cpp | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/coreneuron/gpu/nrn_acc_manager.cpp b/coreneuron/gpu/nrn_acc_manager.cpp index 32477dad0..06d6e328c 100644 --- a/coreneuron/gpu/nrn_acc_manager.cpp +++ b/coreneuron/gpu/nrn_acc_manager.cpp @@ -139,15 +139,6 @@ void setup_nrnthreads_on_device(NrnThread* threads, int nthreads) { acc_memcpy_to_device(&(d_nt->_actual_diam), &(dptr), sizeof(double*)); } - size_t offset = 6 * ne; - - // see Phase2::populate(): some mechanisms can have diam semantics - // added after matrix data in nt._data. Take that into account while - // calculating the offset for start of mechanism data. - if (nt->_actual_diam) { - offset += ne; - } - int* d_v_parent_index = (int*) acc_copyin(nt->_v_parent_index, nt->end * sizeof(int)); acc_memcpy_to_device(&(d_nt->_v_parent_index), &(d_v_parent_index), sizeof(int*)); @@ -194,13 +185,10 @@ void setup_nrnthreads_on_device(NrnThread* threads, int nthreads) { int is_art = corenrn.get_is_artificial()[type]; int layout = corenrn.get_mech_data_layout()[type]; - offset = nrn_soa_padded_size(offset, layout); - - dptr = d__data + offset; - + // get device pointer for corresponding mechanism data + dptr = (double *) acc_deviceptr(tml->ml->data); acc_memcpy_to_device(&(d_ml->data), &(dptr), sizeof(double*)); - offset += nrn_soa_padded_size(n, layout) * szp; if (!is_art) { int* d_nodeindices = (int*) acc_copyin(tml->ml->nodeindices, sizeof(int) * n); From 55b470821ad4e6e54e80ef1a8c363ef3a63e4cfc Mon Sep 17 00:00:00 2001 From: Pramod Kumbhar Date: Fri, 16 Jul 2021 22:08:01 +0200 Subject: [PATCH 3/3] fix clang-format --- coreneuron/gpu/nrn_acc_manager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coreneuron/gpu/nrn_acc_manager.cpp b/coreneuron/gpu/nrn_acc_manager.cpp index 06d6e328c..9454c712f 100644 --- a/coreneuron/gpu/nrn_acc_manager.cpp +++ b/coreneuron/gpu/nrn_acc_manager.cpp @@ -186,7 +186,7 @@ void setup_nrnthreads_on_device(NrnThread* threads, int nthreads) { int layout = corenrn.get_mech_data_layout()[type]; // get device pointer for corresponding mechanism data - dptr = (double *) acc_deviceptr(tml->ml->data); + dptr = (double*) acc_deviceptr(tml->ml->data); acc_memcpy_to_device(&(d_ml->data), &(dptr), sizeof(double*));