GetConnections not working with multiple threads in combination with detailed timers.
Calling GetConnections when using multiple threads triggers the assertion in line 207 of vp_manager.cpp. The following script reproduces the issue:
import nest
nest.total_num_virtual_procs = 2
neuron = nest.Create("parrot_neuron")
nest.Connect(neuron, neuron)
nest.GetConnections()
@JanVogelsang The code works on my computer, but it may depend on build details. How did you build NEST (which cmake options)?
Here is the list of (relevant) cmake options I use: -Dwith-optimize=OFF -Dwith-debug=ON -Dwith-mpi=ON -Dwith-cpp-std="c++17" -Dwith-models="iaf_psc_alpha;static_synapse;poisson_generator;spike_recorder;weight_recorder;stdp_pl_synapse_hom;parrot_neuron;music_event_in_proxy;music_event_out_proxy" -Dwith-detailed-timers=ON -Dwith-mpi-sync-timer=ON
I have localized the problem now. If we have detailed, threaded timers, the stopwatch start here (https://github.com/nest/nest-simulator/blob/09b349c55298d346e8aceea4f0ae643dea8ee7a3/nestkernel/per_thread_bool_indicator.cpp#L84) triggers a check for running in parallel here (https://github.com/nest/nest-simulator/blob/09b349c55298d346e8aceea4f0ae643dea8ee7a3/nestkernel/stopwatch_impl.h#L36) which fails, because we get here from a serial context here (https://github.com/nest/nest-simulator/blob/09b349c55298d346e8aceea4f0ae643dea8ee7a3/nestkernel/connection_manager.cpp#L1227).
I am not yet sure how to resolve this. We are doing something serially here which makes eminent sense to do serially, but then end up in a place that may also be called from a parallel context so the assertion does make sense.
Hm true. We could probably move the omp parallel outside of the if ( not source.get() and not target.get() ) and do the if ( is_source_table_cleared() ) inside the parallel region.
Hm true. We could probably move the
omp paralleloutside of theif ( not source.get() and not target.get() )and do theif ( is_source_table_cleared() )inside the parallel region.
Yes, that should work. We could do the check for 0 connections first, then enter #omp parallel. Then we would also just need the parallel context once outside the entire if-elseif-chain. And when we are at it, we might want to split the function into three, one for each of the three cases we have (the fourth is hidden deep in the third with another if) for better readability, keeping any function shorter than a screen height.
Sounds good! Are you already on it or should I quickly implement it?
It would be great if you could create a PR. I’ll be happy to review 😊.
Best, Hans Ekkehard
--
Prof. Dr. Hans Ekkehard Plesser Research Committee Chair, Faculty of Science and Technology Head, Department of Data Science
Department of Data Science Faculty of Science and Technology Norwegian University of Life Sciences PO Box 5003, 1432 Aas, Norway
Phone +47 6723 1560 Email @.@.> Home http://arken.nmbu.no/~plesser
From: Jan Vogelsang @.> Date: Wednesday, 26 March 2025 at 10:14 To: nest/nest-simulator @.> Cc: Hans Ekkehard Plesser @.>, Comment @.> Subject: Re: [nest/nest-simulator] GetConnections not working with multiple threads (Issue #3442)
Sounds good! Are you already on it or should I quickly implement it?
— Reply to this email directly, view it on GitHubhttps://github.com/nest/nest-simulator/issues/3442#issuecomment-2753690965, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAICWPGEC5676G7XVTT44JD2WJOUPAVCNFSM6AAAAABZXJERHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJTGY4TAOJWGU. You are receiving this because you commented.Message ID: @.***> [Image removed by sender. JanVogelsang]JanVogelsang left a comment (nest/nest-simulator#3442)https://github.com/nest/nest-simulator/issues/3442#issuecomment-2753690965
Sounds good! Are you already on it or should I quickly implement it?
— Reply to this email directly, view it on GitHubhttps://github.com/nest/nest-simulator/issues/3442#issuecomment-2753690965, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAICWPGEC5676G7XVTT44JD2WJOUPAVCNFSM6AAAAABZXJERHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJTGY4TAOJWGU. You are receiving this because you commented.Message ID: @.***>
Done, see PR #3445. I also added a small regression test.
@JanVogelsang Why did you close this already, given that the PR is not yet merged? Re-open?
Lowered priority and added CI because this issue was not detected by our CI.
Issue automatically marked stale!
Fixed by #3445.