diff --git a/.github/workflows/nestbuildmatrix.yml b/.github/workflows/nestbuildmatrix.yml index bff990124c..1abde3b8f6 100644 --- a/.github/workflows/nestbuildmatrix.yml +++ b/.github/workflows/nestbuildmatrix.yml @@ -527,7 +527,7 @@ jobs: - "boost, optimize, warning" - "openmp, python, gsl, ltdl, boost, optimize, warning" - "mpi, python, gsl, ltdl, boost, optimize, warning" - - "openmp, mpi, python, gsl, ltdl, boost, hdf5, sionlib, libneurosim, optimize, warning, music" + - "openmp, mpi, python, gsl, ltdl, boost, hdf5, sionlib, libneurosim, optimize, warning, music, detailed-timers, mpi-sync-timer" steps: - name: Harden Runner @@ -679,6 +679,8 @@ jobs: -Dwith-sionlib=${{ contains(matrix.use, 'sionlib') && '$HOME/.cache/sionlib.install' || 'OFF' }} \ -Dwith-libneurosim=${{ contains(matrix.use, 'libneurosim') && '$HOME/.cache/libneurosim.install' || 'OFF' }} \ -Dwith-music=${{ contains(matrix.use, 'music') && '$HOME/.cache/music.install' || 'OFF' }} \ + -Dwith-detailed-timers=${{ contains(matrix.use, 'detailed-timers') && 'ON' || 'OFF' }} \ + -Dwith-mpi-sync-timer=${{ contains(matrix.use, 'mpi-sync-timer') && 'ON' || 'OFF' }} \ .. - name: "Add GCC problem matcher" diff --git a/nestkernel/connection_manager.cpp b/nestkernel/connection_manager.cpp index 7228915296..0924fb73e2 100644 --- a/nestkernel/connection_manager.cpp +++ b/nestkernel/connection_manager.cpp @@ -1218,188 +1218,182 @@ nest::ConnectionManager::split_to_neuron_device_vectors_( const size_t tid, } void -nest::ConnectionManager::get_connections( std::deque< ConnectionID >& connectome, - NodeCollectionPTR source, - NodeCollectionPTR target, +nest::ConnectionManager::get_connections_( const size_t tid, + std::deque< ConnectionID >& conns_in_thread, + NodeCollectionPTR, + NodeCollectionPTR, synindex syn_id, long synapse_label ) const { - if ( is_source_table_cleared() ) + ConnectorBase* connections = connections_[ tid ][ syn_id ]; + if ( connections ) { - throw KernelException( - "Invalid attempt to access connection information: source table was " - "cleared." ); + // Passing target_node_id = 0 ignores target_node_id while getting connections. + const size_t num_connections_in_thread = connections->size(); + for ( size_t lcid = 0; lcid < num_connections_in_thread; ++lcid ) + { + const size_t source_node_id = source_table_.get_node_id( tid, syn_id, lcid ); + connections->get_connection( source_node_id, 0, tid, lcid, synapse_label, conns_in_thread ); + } } - const size_t num_connections = get_num_connections( syn_id ); + target_table_devices_.get_connections( 0, 0, tid, syn_id, synapse_label, conns_in_thread ); +} - if ( num_connections == 0 ) - { - return; - } +void +nest::ConnectionManager::get_connections_to_targets_( const size_t tid, + std::deque< ConnectionID >& conns_in_thread, + NodeCollectionPTR, + NodeCollectionPTR target, + synindex syn_id, + long synapse_label ) const +{ + // Split targets into neuron- and device-vectors. + std::vector< size_t > target_neuron_node_ids; + std::vector< size_t > target_device_node_ids; + split_to_neuron_device_vectors_( tid, target, target_neuron_node_ids, target_device_node_ids ); - if ( not source.get() and not target.get() ) + // Getting regular connections, if they exist. + ConnectorBase* connections = connections_[ tid ][ syn_id ]; + if ( connections ) { -#pragma omp parallel + const size_t num_connections_in_thread = connections->size(); + for ( size_t lcid = 0; lcid < num_connections_in_thread; ++lcid ) { - size_t tid = kernel().vp_manager.get_thread_id(); + const size_t source_node_id = source_table_.get_node_id( tid, syn_id, lcid ); + connections->get_connection_with_specified_targets( + source_node_id, target_neuron_node_ids, tid, lcid, synapse_label, conns_in_thread ); + } + } - std::deque< ConnectionID > conns_in_thread; + // Getting connections from devices. + for ( auto t_node_id : target_neuron_node_ids ) + { + target_table_devices_.get_connections_from_devices_( 0, t_node_id, tid, syn_id, synapse_label, conns_in_thread ); + } - ConnectorBase* connections = connections_[ tid ][ syn_id ]; - if ( connections ) - { - // Passing target_node_id = 0 ignores target_node_id while getting connections. - const size_t num_connections_in_thread = connections->size(); - for ( size_t lcid = 0; lcid < num_connections_in_thread; ++lcid ) - { - const size_t source_node_id = source_table_.get_node_id( tid, syn_id, lcid ); - connections->get_connection( source_node_id, 0, tid, lcid, synapse_label, conns_in_thread ); - } - } + // Getting connections to devices. + for ( auto t_device_id : target_device_node_ids ) + { + target_table_devices_.get_connections_to_devices_( 0, t_device_id, tid, syn_id, synapse_label, conns_in_thread ); + } +} - target_table_devices_.get_connections( 0, 0, tid, syn_id, synapse_label, conns_in_thread ); +void +nest::ConnectionManager::get_connections_from_sources_( const size_t tid, + std::deque< ConnectionID >& conns_in_thread, + NodeCollectionPTR source, + NodeCollectionPTR target, + synindex syn_id, + long synapse_label ) const +{ + // Split targets into neuron- and device-vectors. + std::vector< size_t > target_neuron_node_ids; + std::vector< size_t > target_device_node_ids; + if ( target.get() ) + { + split_to_neuron_device_vectors_( tid, target, target_neuron_node_ids, target_device_node_ids ); + } - if ( conns_in_thread.size() > 0 ) - { -#pragma omp critical( get_connections ) - { - extend_connectome( connectome, conns_in_thread ); - } - } - } // of omp parallel - return; - } // if - else if ( not source.get() and target.get() ) + const ConnectorBase* connections = connections_[ tid ][ syn_id ]; + if ( connections ) { -#pragma omp parallel + const size_t num_connections_in_thread = connections->size(); + for ( size_t lcid = 0; lcid < num_connections_in_thread; ++lcid ) { - size_t tid = kernel().vp_manager.get_thread_id(); - - std::deque< ConnectionID > conns_in_thread; - - // Split targets into neuron- and device-vectors. - std::vector< size_t > target_neuron_node_ids; - std::vector< size_t > target_device_node_ids; - split_to_neuron_device_vectors_( tid, target, target_neuron_node_ids, target_device_node_ids ); - - // Getting regular connections, if they exist. - ConnectorBase* connections = connections_[ tid ][ syn_id ]; - if ( connections ) + const size_t source_node_id = source_table_.get_node_id( tid, syn_id, lcid ); + if ( source->contains( source_node_id ) ) { - const size_t num_connections_in_thread = connections->size(); - for ( size_t lcid = 0; lcid < num_connections_in_thread; ++lcid ) + if ( not target.get() ) + { + // Passing target_node_id = 0 ignores target_node_id while getting + // connections. + connections->get_connection( source_node_id, 0, tid, lcid, synapse_label, conns_in_thread ); + } + else { - const size_t source_node_id = source_table_.get_node_id( tid, syn_id, lcid ); connections->get_connection_with_specified_targets( source_node_id, target_neuron_node_ids, tid, lcid, synapse_label, conns_in_thread ); } } + } + } - // Getting connections from devices. - for ( auto t_node_id : target_neuron_node_ids ) + NodeCollection::const_iterator s_id = source->begin(); + for ( ; s_id < source->end(); ++s_id ) + { + const size_t source_node_id = ( *s_id ).node_id; + if ( not target.get() ) + { + target_table_devices_.get_connections( source_node_id, 0, tid, syn_id, synapse_label, conns_in_thread ); + } + else + { + for ( std::vector< size_t >::const_iterator t_node_id = target_neuron_node_ids.begin(); + t_node_id != target_neuron_node_ids.end(); + ++t_node_id ) { + // target_table_devices_ contains connections both to and from + // devices. First we get connections from devices. target_table_devices_.get_connections_from_devices_( - 0, t_node_id, tid, syn_id, synapse_label, conns_in_thread ); + source_node_id, *t_node_id, tid, syn_id, synapse_label, conns_in_thread ); } - - // Getting connections to devices. - for ( auto t_device_id : target_device_node_ids ) + for ( std::vector< size_t >::const_iterator t_node_id = target_device_node_ids.begin(); + t_node_id != target_device_node_ids.end(); + ++t_node_id ) { + // Then, we get connections to devices. target_table_devices_.get_connections_to_devices_( - 0, t_device_id, tid, syn_id, synapse_label, conns_in_thread ); + source_node_id, *t_node_id, tid, syn_id, synapse_label, conns_in_thread ); } + } + } +} - if ( conns_in_thread.size() > 0 ) - { -#pragma omp critical( get_connections ) - { - extend_connectome( connectome, conns_in_thread ); - } - } - } // of omp parallel - return; - } // else if - else if ( source.get() ) +void +nest::ConnectionManager::get_connections( std::deque< ConnectionID >& connectome, + NodeCollectionPTR source, + NodeCollectionPTR target, + synindex syn_id, + long synapse_label ) const +{ + if ( get_num_connections( syn_id ) == 0 ) { + return; + } + #pragma omp parallel + { + if ( is_source_table_cleared() ) { - size_t tid = kernel().vp_manager.get_thread_id(); + throw KernelException( "Invalid attempt to access connection information: source table was cleared." ); + } - std::deque< ConnectionID > conns_in_thread; + size_t tid = kernel().vp_manager.get_thread_id(); - // Split targets into neuron- and device-vectors. - std::vector< size_t > target_neuron_node_ids; - std::vector< size_t > target_device_node_ids; - if ( target.get() ) - { - split_to_neuron_device_vectors_( tid, target, target_neuron_node_ids, target_device_node_ids ); - } + std::deque< ConnectionID > conns_in_thread; - const ConnectorBase* connections = connections_[ tid ][ syn_id ]; - if ( connections ) - { - const size_t num_connections_in_thread = connections->size(); - for ( size_t lcid = 0; lcid < num_connections_in_thread; ++lcid ) - { - const size_t source_node_id = source_table_.get_node_id( tid, syn_id, lcid ); - if ( source->contains( source_node_id ) ) - { - if ( not target.get() ) - { - // Passing target_node_id = 0 ignores target_node_id while getting - // connections. - connections->get_connection( source_node_id, 0, tid, lcid, synapse_label, conns_in_thread ); - } - else - { - connections->get_connection_with_specified_targets( - source_node_id, target_neuron_node_ids, tid, lcid, synapse_label, conns_in_thread ); - } - } - } - } - - NodeCollection::const_iterator s_id = source->begin(); - for ( ; s_id < source->end(); ++s_id ) - { - const size_t source_node_id = ( *s_id ).node_id; - if ( not target.get() ) - { - target_table_devices_.get_connections( source_node_id, 0, tid, syn_id, synapse_label, conns_in_thread ); - } - else - { - for ( std::vector< size_t >::const_iterator t_node_id = target_neuron_node_ids.begin(); - t_node_id != target_neuron_node_ids.end(); - ++t_node_id ) - { - // target_table_devices_ contains connections both to and from - // devices. First we get connections from devices. - target_table_devices_.get_connections_from_devices_( - source_node_id, *t_node_id, tid, syn_id, synapse_label, conns_in_thread ); - } - for ( std::vector< size_t >::const_iterator t_node_id = target_device_node_ids.begin(); - t_node_id != target_device_node_ids.end(); - ++t_node_id ) - { - // Then, we get connections to devices. - target_table_devices_.get_connections_to_devices_( - source_node_id, *t_node_id, tid, syn_id, synapse_label, conns_in_thread ); - } - } - } + if ( not source.get() and not target.get() ) + { + get_connections_( tid, conns_in_thread, source, target, syn_id, synapse_label ); + } + else if ( not source.get() and target.get() ) + { + get_connections_to_targets_( tid, conns_in_thread, source, target, syn_id, synapse_label ); + } + else if ( source.get() ) + { + get_connections_from_sources_( tid, conns_in_thread, source, target, syn_id, synapse_label ); + } - if ( conns_in_thread.size() > 0 ) - { + if ( conns_in_thread.size() > 0 ) + { #pragma omp critical( get_connections ) - { - extend_connectome( connectome, conns_in_thread ); - } + { + extend_connectome( connectome, conns_in_thread ); } - } // of omp parallel - return; - } // else if + } + } } void diff --git a/nestkernel/connection_manager.h b/nestkernel/connection_manager.h index 49a8a8ca39..ca0c07d11d 100644 --- a/nestkernel/connection_manager.h +++ b/nestkernel/connection_manager.h @@ -473,6 +473,26 @@ class ConnectionManager : public ManagerInterface size_t get_num_connections_( const size_t tid, const synindex syn_id ) const; + //! See get_connections() + void get_connections_( const size_t tid, + std::deque< ConnectionID >& connectome, + NodeCollectionPTR source, + NodeCollectionPTR target, + synindex syn_id, + long synapse_label ) const; + void get_connections_to_targets_( const size_t tid, + std::deque< ConnectionID >& connectome, + NodeCollectionPTR source, + NodeCollectionPTR target, + synindex syn_id, + long synapse_label ) const; + void get_connections_from_sources_( const size_t tid, + std::deque< ConnectionID >& connectome, + NodeCollectionPTR source, + NodeCollectionPTR target, + synindex syn_id, + long synapse_label ) const; + void get_source_node_ids_( const size_t tid, const synindex syn_id, const size_t tnode_id, diff --git a/testsuite/pytests/test_issue_3442.py b/testsuite/pytests/test_issue_3442.py new file mode 100644 index 0000000000..de5050dcce --- /dev/null +++ b/testsuite/pytests/test_issue_3442.py @@ -0,0 +1,43 @@ +# -*- coding: utf-8 -*- +# +# test_issue_3442.py +# +# This file is part of NEST. +# +# Copyright (C) 2004 The NEST Initiative +# +# NEST is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 2 of the License, or +# (at your option) any later version. +# +# NEST is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with NEST. If not, see . + + +import nest +import pytest + +""" +Tests that get_connections works if called when also using multithreading. + +This test would only fail before fixing #3442 if NEST was built with `-Dwith-detailed-timers=ON`. +""" + + +@pytest.mark.skipif_missing_threads +@pytest.mark.parametrize("n_threads", [1, 2, 4]) +def test_threaded_get_connections(n_threads): + nest.ResetKernel() + + nest.total_num_virtual_procs = n_threads + + neuron = nest.Create("parrot_neuron") + nest.Connect(neuron, neuron) + + nest.GetConnections()