If a netcon_srcgid is negative, need to determine the thread. #390
Conversation
Only for direct transfer mode is it allowed that a negative srcgid is not in the same thread as the NetCon target. To enable thread determination an std::vector<int> involving the name netcon_negsrcgid_tid is associated with netcon_srcgid in that when a negative gid appears in netcon_srcgid, the tid is the value of the element in netcon_negsrcgid_tid.
|
A stack trace from the BBP pipeline: |
It puzzles me how this can be NULL since line 291 and prior to Phase1 processing. Shouldn't that have made a nrn_nthread size vector of empty vectors? |
Absolutely. My guess is that it doesn't get called. I can have a quick look, I have CoreNeuron set up. |
|
|
|
c870bf3 should work but I would prefer avoiding the additional clutter by just creating an empty vector of vectors for |
I know it's more verbose, just wanted to be closer to your original logic of resizing only for direct mode. Then I realized it makes things more obvious if we linked We can revert back to 7af3e93 if you prefer, however in that case we would need to keep the following (because diff --git a/coreneuron/io/phase1.cpp b/coreneuron/io/phase1.cpp
index fd5d3287..eff0dc4c 100644
--- a/coreneuron/io/phase1.cpp
+++ b/coreneuron/io/phase1.cpp
@@ -58,6 +58,10 @@ void Phase1::read_direct(int thread_id) {
delete[] netcon_srcgid;
}
+extern "C" {
+extern bool corenrn_embedded;
+}
+
void Phase1::populate(NrnThread& nt, OMP_Mutex& mut) {
nt.n_presyn = this->output_gids.size();
nt.n_netcon = this->netcon_srcgids.size();
@@ -66,7 +70,7 @@ void Phase1::populate(NrnThread& nt, OMP_Mutex& mut) {
std::copy(this->netcon_srcgids.begin(), this->netcon_srcgids.end(),
netcon_srcgid[nt.id]);
- if (!coreneuron::netcon_negsrcgid_tid.empty()) { // multiple threads and direct mode.
+ if (corenrn_embedded) { // multiple threads and direct mode.
coreneuron::netcon_negsrcgid_tid[nt.id] = this->netcon_negsrcgid_tid;
} |
|
@alexsavulescu @nrnhines : changes look good to me. But I stumble on this: std::vector<int> dummy;
std::vector<int>& negsrcgid_tid = corenrn_embedded ? nrnthreads_netcon_negsrcgid_tid[ith] : dummy;
...
if (!negsrcgid_tid.empty()) {
tid = negsrcgid_tid[i_tid++];
}if anyway we have to protect access to if (corenrn_embedded) {
tid = nrnthreads_netcon_negsrcgid_tid[ith][i_tid++];
}which was this comment #390 (comment) I think. (reference to empty |
|
I like that suggestion. Good balance between @alexsavulescu point about "closer to your original logic of resizing only for direct mode." and just creatiing a vector of empty vectors. But I guess we still need to create the vector of empty vectors for the file mode a la 7af3e93, otherwise how can one safely as opposed to Nevermind, I didn't notice your suggesting factoring in Darn... There is a subtle bug when replacing with and that is the fact that nrnthreads_netcon_negsrcgid_tid is not set up on the NEURON side when |
Comments indicate that nrnthreads_netcon_negsrc_gid_tid subvectors are definitely empty when single thread or file transfer. (will be empty anyway if there are no negative gids) This reverts commit c870bf3.
|
Given that |
…390 (#392) * Implement support for version from last git commit - coreneuron version was printed as "version id unimplemented" - add cmake module GitRevision.cmake to find git commit information - this commit adds support to print project version along with git commit and it's date. e.g. Version : 0.21.0 bd747a8 (14-09-2020 16:03) * Added --version CLI flag - Use git show instead of git show to be compatible with older git (e.g. on BB5) * Avoid nrn_abort when --help is called * Bug fix for merged pull request #390 : tid and ith args were mixed * For BB5 Jenkins, exclude problematic agent bb5-07 (has strange PGI compiler issue) Co-authored-by: Michael Hines <[email protected]>
…ain/CoreNeuron#390) * If a netcon_srcgid is negative, need to determine the thread. Only for direct transfer mode is it allowed that a negative srcgid is not in the same thread as the NetCon target. To enable thread determination an std::vector<int> involving the name netcon_negsrcgid_tid is associated with netcon_srcgid in that when a negative gid appears in netcon_srcgid, the tid is the value of the element in netcon_negsrcgid_tid. * always resize netcon_negsrcgid_tid * link netcon_negsrcgid_tid usage to corenrn_embedded * nrnthreads_netcon_srcgid is more meaninfgul name. * Revert "link netcon_negsrcgid_tid usage to corenrn_embedded" Comments indicate that nrnthreads_netcon_negsrc_gid_tid subvectors are definitely empty when single thread or file transfer. (will be empty anyway if there are no negative gids) This reverts commit c870bf32b03e936654394789a48e9a0bdb749a13. Co-authored-by: Alexandru Savulescu <[email protected]> CoreNEURON Repo SHA: BlueBrain/CoreNeuron@bd747a8
…lueBrain/CoreNeuron#390 (BlueBrain/CoreNeuron#392) * Implement support for version from last git commit - coreneuron version was printed as "version id unimplemented" - add cmake module GitRevision.cmake to find git commit information - this commit adds support to print project version along with git commit and it's date. e.g. Version : 0.21.0 bd747a8e (14-09-2020 16:03) * Added --version CLI flag - Use git show instead of git show to be compatible with older git (e.g. on BB5) * Avoid nrn_abort when --help is called * Bug fix for merged pull request BlueBrain/CoreNeuron#390 : tid and ith args were mixed * For BB5 Jenkins, exclude problematic agent bb5-07 (has strange PGI compiler issue) Co-authored-by: Michael Hines <[email protected]> CoreNEURON Repo SHA: BlueBrain/CoreNeuron@e0a2774
Only for direct transfer mode is it allowed that a negative srcgid is not
in the same thread as the NetCon target.
To enable thread determination an std::vector involving the name
netcon_negsrcgid_tid is associated with netcon_srcgid in that when a negative
gid appears in netcon_srcgid, the tid is the value of the element in
netcon_negsrcgid_tid.
This pr is assciated with neuronsimulator/nrn#735