From 35e61a8c0f988cf9d7e783864486693209db3202 Mon Sep 17 00:00:00 2001 From: xavierotazuGDS Date: Mon, 18 Mar 2024 16:07:22 +0100 Subject: [PATCH 01/11] Update layer_impl.h Instead of calling get_connections() inside a loop for every source node, it calls get_connections() once for all connections between layers. The code considers that: 1) source nodes positions (src_vec) is ordered by source node. 2) connectome is ordered by source node. --- nestkernel/layer_impl.h | 42 ++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/nestkernel/layer_impl.h b/nestkernel/layer_impl.h index 5f44de8527..3e9b70ac52 100644 --- a/nestkernel/layer_impl.h +++ b/nestkernel/layer_impl.h @@ -303,33 +303,38 @@ Layer< D >::dump_connections( std::ostream& out, AbstractLayerPTR target_layer, const Token& syn_model ) { - std::vector< std::pair< Position< D >, size_t > >* src_vec = get_global_positions_vector( node_collection ); + std::vector< std::pair< Position< D >, size_t > >* src_vec = get_global_positions_vector( node_collection ); - // Dictionary with parameters for get_connections() - DictionaryDatum conn_filter( new Dictionary ); - def( conn_filter, names::synapse_model, syn_model ); - def( conn_filter, names::target, NodeCollectionDatum( target_layer->get_node_collection() ) ); + // Dictionary with parameters for get_connections() + DictionaryDatum conn_filter( new Dictionary ); + def( conn_filter, names::synapse_model, syn_model ); + def( conn_filter, names::target, NodeCollectionDatum( target_layer->get_node_collection() ) ); + def( conn_filter, names::source, NodeCollectionDatum( node_collection ) ); - // Avoid setting up new array for each iteration of the loop - std::vector< size_t > source_array( 1 ); + ArrayDatum connectome = kernel().connection_manager.get_connections( conn_filter ); - for ( typename std::vector< std::pair< Position< D >, size_t > >::iterator src_iter = src_vec->begin(); - src_iter != src_vec->end(); - ++src_iter ) - { + // Initialize iterator and variables + size_t previous_source_node_id = getValue< ConnectionDatum >( connectome.get( 0 ) ).get_source_node_id(); + typename std::vector< std::pair< Position< D >, size_t > >::iterator src_iter = src_vec->begin(); + Position< D > source_pos = src_iter->first; - const size_t source_node_id = src_iter->second; - const Position< D > source_pos = src_iter->first; - - source_array[ 0 ] = source_node_id; - def( conn_filter, names::source, NodeCollectionDatum( NodeCollection::create( source_array ) ) ); - ArrayDatum connectome = kernel().connection_manager.get_connections( conn_filter ); // Print information about all local connections for current source for ( size_t i = 0; i < connectome.size(); ++i ) { ConnectionDatum con_id = getValue< ConnectionDatum >( connectome.get( i ) ); - DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( con_id.get_source_node_id(), + const size_t source_node_id = con_id.get_source_node_id(); + + // Search source_pos for source node only if it is a different node. It considers both source nodes positions (src_vec) and connectome are ordered by source node + if(source_node_id != previous_source_node_id) + { + assert((src_iter++)!=src_vec->end()); + source_pos = src_iter->first; + + previous_source_node_id = source_node_id; + } + + DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( source_node_id, con_id.get_target_node_id(), con_id.get_target_thread(), con_id.get_synapse_model_id(), @@ -350,7 +355,6 @@ Layer< D >::dump_connections( std::ostream& out, tgt_layer->compute_displacement( source_pos, tnode_lid ).print( out ); out << '\n'; } - } } template < int D > From 90bf3abc9246c625336b7d4457bf9d6ac64a8f75 Mon Sep 17 00:00:00 2001 From: xavierotazuGDS Date: Tue, 19 Mar 2024 00:24:38 +0100 Subject: [PATCH 02/11] Update layer_impl.h New version. No bugs. It is a more "conservative" approach, since it does not consider nodes are ordered. --- nestkernel/layer_impl.h | 73 +++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/nestkernel/layer_impl.h b/nestkernel/layer_impl.h index 3e9b70ac52..6f1b75af74 100644 --- a/nestkernel/layer_impl.h +++ b/nestkernel/layer_impl.h @@ -303,58 +303,59 @@ Layer< D >::dump_connections( std::ostream& out, AbstractLayerPTR target_layer, const Token& syn_model ) { - std::vector< std::pair< Position< D >, size_t > >* src_vec = get_global_positions_vector( node_collection ); + std::vector< std::pair< Position< D >, size_t > >* src_vec = get_global_positions_vector( node_collection ); - // Dictionary with parameters for get_connections() - DictionaryDatum conn_filter( new Dictionary ); - def( conn_filter, names::synapse_model, syn_model ); - def( conn_filter, names::target, NodeCollectionDatum( target_layer->get_node_collection() ) ); - def( conn_filter, names::source, NodeCollectionDatum( node_collection ) ); + // Dictionary with parameters for get_connections() + DictionaryDatum conn_filter( new Dictionary ); + def( conn_filter, names::synapse_model, syn_model ); + def( conn_filter, names::target, NodeCollectionDatum( target_layer->get_node_collection() ) ); + def( conn_filter, names::source, NodeCollectionDatum( node_collection ) ); - ArrayDatum connectome = kernel().connection_manager.get_connections( conn_filter ); + ArrayDatum connectome = kernel().connection_manager.get_connections( conn_filter ); - // Initialize iterator and variables - size_t previous_source_node_id = getValue< ConnectionDatum >( connectome.get( 0 ) ).get_source_node_id(); - typename std::vector< std::pair< Position< D >, size_t > >::iterator src_iter = src_vec->begin(); - Position< D > source_pos = src_iter->first; + size_t previous_source_node_id = getValue< ConnectionDatum >( connectome.get( 0 ) ).get_source_node_id(); + Position< D > source_pos = src_vec->begin()->first; + // Print information about all local connections for current source + for ( size_t i = 0; i < connectome.size(); ++i ) + { + ConnectionDatum con_id = getValue< ConnectionDatum >( connectome.get( i ) ); + const size_t source_node_id = con_id.get_source_node_id(); - // Print information about all local connections for current source - for ( size_t i = 0; i < connectome.size(); ++i ) + // Search source_pos for source node only if it is a different node + if(source_node_id != previous_source_node_id) { - ConnectionDatum con_id = getValue< ConnectionDatum >( connectome.get( i ) ); - const size_t source_node_id = con_id.get_source_node_id(); + source_pos = src_vec->begin()->first; - // Search source_pos for source node only if it is a different node. It considers both source nodes positions (src_vec) and connectome are ordered by source node - if(source_node_id != previous_source_node_id) - { - assert((src_iter++)!=src_vec->end()); - source_pos = src_iter->first; - - previous_source_node_id = source_node_id; - } + for ( typename std::vector< std::pair< Position< D >, size_t > >::iterator src_iter = src_vec->begin(); + src_iter != src_vec->end() && source_node_id!=src_iter->second; + ++src_iter, source_pos = src_iter->first); - DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( source_node_id, + previous_source_node_id = source_node_id; + } + +// DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( con_id.get_source_node_id(), + DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( source_node_id, con_id.get_target_node_id(), con_id.get_target_thread(), con_id.get_synapse_model_id(), con_id.get_port() ); - long target_node_id = getValue< long >( result_dict, names::target ); - double weight = getValue< double >( result_dict, names::weight ); - double delay = getValue< double >( result_dict, names::delay ); + long target_node_id = getValue< long >( result_dict, names::target ); + double weight = getValue< double >( result_dict, names::weight ); + double delay = getValue< double >( result_dict, names::delay ); - // Print source, target, weight, delay, rports - out << source_node_id << ' ' << target_node_id << ' ' << weight << ' ' << delay; + // Print source, target, weight, delay, rports + out << source_node_id << ' ' << target_node_id << ' ' << weight << ' ' << delay; - Layer< D >* tgt_layer = dynamic_cast< Layer< D >* >( target_layer.get() ); + Layer< D >* tgt_layer = dynamic_cast< Layer< D >* >( target_layer.get() ); + + out << ' '; + const long tnode_lid = tgt_layer->node_collection_->get_lid( target_node_id ); + assert( tnode_lid >= 0 ); + tgt_layer->compute_displacement( source_pos, tnode_lid ).print( out ); + out << '\n'; - out << ' '; - const long tnode_lid = tgt_layer->node_collection_->get_lid( target_node_id ); - assert( tnode_lid >= 0 ); - tgt_layer->compute_displacement( source_pos, tnode_lid ).print( out ); - out << '\n'; - } } template < int D > From 7ec42eefb720e1d870a9b163536273e1ea242b8d Mon Sep 17 00:00:00 2001 From: xavierotazuGDS Date: Wed, 20 Mar 2024 14:06:50 +0100 Subject: [PATCH 03/11] clang-format style Code style modified to agree with clang-format --- nestkernel/layer_impl.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nestkernel/layer_impl.h b/nestkernel/layer_impl.h index 6f1b75af74..b624099ab7 100644 --- a/nestkernel/layer_impl.h +++ b/nestkernel/layer_impl.h @@ -313,6 +313,7 @@ Layer< D >::dump_connections( std::ostream& out, ArrayDatum connectome = kernel().connection_manager.get_connections( conn_filter ); + // Get variables for loop size_t previous_source_node_id = getValue< ConnectionDatum >( connectome.get( 0 ) ).get_source_node_id(); Position< D > source_pos = src_vec->begin()->first; @@ -323,18 +324,17 @@ Layer< D >::dump_connections( std::ostream& out, const size_t source_node_id = con_id.get_source_node_id(); // Search source_pos for source node only if it is a different node - if(source_node_id != previous_source_node_id) + if( source_node_id != previous_source_node_id ) { source_pos = src_vec->begin()->first; for ( typename std::vector< std::pair< Position< D >, size_t > >::iterator src_iter = src_vec->begin(); - src_iter != src_vec->end() && source_node_id!=src_iter->second; - ++src_iter, source_pos = src_iter->first); + src_iter != src_vec->end() && source_node_id! = src_iter->second; + ++src_iter, source_pos = src_iter->first ); previous_source_node_id = source_node_id; } -// DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( con_id.get_source_node_id(), DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( source_node_id, con_id.get_target_node_id(), con_id.get_target_thread(), From b7fc71c86d9f09db962dc0c87d3b9eddcf1262e5 Mon Sep 17 00:00:00 2001 From: xavierotazuGDS Date: Wed, 20 Mar 2024 15:05:52 +0100 Subject: [PATCH 04/11] clang-format v2 --- nestkernel/layer_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nestkernel/layer_impl.h b/nestkernel/layer_impl.h index b624099ab7..78e588fbb8 100644 --- a/nestkernel/layer_impl.h +++ b/nestkernel/layer_impl.h @@ -332,7 +332,7 @@ Layer< D >::dump_connections( std::ostream& out, src_iter != src_vec->end() && source_node_id! = src_iter->second; ++src_iter, source_pos = src_iter->first ); - previous_source_node_id = source_node_id; + previous_source_node_id = source_node_id; } DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( source_node_id, From 2d328069fa09b084d7c6856b07e81e2f4638b86e Mon Sep 17 00:00:00 2001 From: xavierotazuGDS Date: Wed, 20 Mar 2024 15:10:39 +0100 Subject: [PATCH 05/11] clang-format v3 Automatic clang-format checking complains about functions I have not modified. --- nestkernel/layer_impl.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/nestkernel/layer_impl.h b/nestkernel/layer_impl.h index 78e588fbb8..a9625d8b3d 100644 --- a/nestkernel/layer_impl.h +++ b/nestkernel/layer_impl.h @@ -324,22 +324,23 @@ Layer< D >::dump_connections( std::ostream& out, const size_t source_node_id = con_id.get_source_node_id(); // Search source_pos for source node only if it is a different node - if( source_node_id != previous_source_node_id ) + if ( source_node_id != previous_source_node_id ) { source_pos = src_vec->begin()->first; for ( typename std::vector< std::pair< Position< D >, size_t > >::iterator src_iter = src_vec->begin(); - src_iter != src_vec->end() && source_node_id! = src_iter->second; - ++src_iter, source_pos = src_iter->first ); + ( src_iter != src_vec->end() ) && ( source_node_id! = src_iter->second ); + ++src_iter, source_pos = src_iter->first ) + ; previous_source_node_id = source_node_id; } DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( source_node_id, - con_id.get_target_node_id(), - con_id.get_target_thread(), - con_id.get_synapse_model_id(), - con_id.get_port() ); + con_id.get_target_node_id(), + con_id.get_target_thread(), + con_id.get_synapse_model_id(), + con_id.get_port() ); long target_node_id = getValue< long >( result_dict, names::target ); double weight = getValue< double >( result_dict, names::weight ); From 3c0e9e2bd093be04b6c56634810dcb5734e27c9e Mon Sep 17 00:00:00 2001 From: xavierotazuGDS Date: Wed, 20 Mar 2024 16:04:25 +0100 Subject: [PATCH 06/11] clang-format --- nestkernel/layer_impl.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nestkernel/layer_impl.h b/nestkernel/layer_impl.h index a9625d8b3d..6ef217ddd4 100644 --- a/nestkernel/layer_impl.h +++ b/nestkernel/layer_impl.h @@ -329,7 +329,7 @@ Layer< D >::dump_connections( std::ostream& out, source_pos = src_vec->begin()->first; for ( typename std::vector< std::pair< Position< D >, size_t > >::iterator src_iter = src_vec->begin(); - ( src_iter != src_vec->end() ) && ( source_node_id! = src_iter->second ); + ( src_iter != src_vec->end() ) && ( source_node_id != src_iter->second ); ++src_iter, source_pos = src_iter->first ) ; @@ -356,7 +356,6 @@ Layer< D >::dump_connections( std::ostream& out, assert( tnode_lid >= 0 ); tgt_layer->compute_displacement( source_pos, tnode_lid ).print( out ); out << '\n'; - } template < int D > From 382c64a83df7483a18f4b9b5af47cbda29acc675 Mon Sep 17 00:00:00 2001 From: xavierotazuGDS Date: Mon, 25 Mar 2024 16:25:01 +0100 Subject: [PATCH 07/11] missing bracket --- nestkernel/layer_impl.h | 1 + 1 file changed, 1 insertion(+) diff --git a/nestkernel/layer_impl.h b/nestkernel/layer_impl.h index 6ef217ddd4..8d9b888612 100644 --- a/nestkernel/layer_impl.h +++ b/nestkernel/layer_impl.h @@ -356,6 +356,7 @@ Layer< D >::dump_connections( std::ostream& out, assert( tnode_lid >= 0 ); tgt_layer->compute_displacement( source_pos, tnode_lid ).print( out ); out << '\n'; + } } template < int D > From f26b359a688e9b3ad9eaa8722588bdc4f8460ae9 Mon Sep 17 00:00:00 2001 From: Hans Ekkehard Plesser Date: Fri, 21 Jun 2024 00:26:36 +0200 Subject: [PATCH 08/11] Replace manual search with find_if() and order code more clearly --- nestkernel/layer_impl.h | 42 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/nestkernel/layer_impl.h b/nestkernel/layer_impl.h index 8f350af583..979764146d 100644 --- a/nestkernel/layer_impl.h +++ b/nestkernel/layer_impl.h @@ -327,40 +327,34 @@ Layer< D >::dump_connections( std::ostream& out, // Print information about all local connections for current source for ( size_t i = 0; i < connectome.size(); ++i ) { - ConnectionDatum con_id = getValue< ConnectionDatum >( connectome.get( i ) ); - const size_t source_node_id = con_id.get_source_node_id(); + ConnectionDatum conn = getValue< ConnectionDatum >( connectome.get( i ) ); + const size_t source_node_id = conn.get_source_node_id(); // Search source_pos for source node only if it is a different node if ( source_node_id != previous_source_node_id ) { - source_pos = src_vec->begin()->first; - - for ( typename std::vector< std::pair< Position< D >, size_t > >::iterator src_iter = src_vec->begin(); - ( src_iter != src_vec->end() ) && ( source_node_id != src_iter->second ); - ++src_iter, source_pos = src_iter->first ) - ; - + const auto it = std::find_if( src_vec->begin(), + src_vec->end(), + [ source_node_id ]( const std::pair< Position< D >, size_t >& p ) { return p.second == source_node_id; } ); + assert( it != src_vec->end() ); + source_pos = it->first; previous_source_node_id = source_node_id; } DictionaryDatum result_dict = kernel().connection_manager.get_synapse_status( source_node_id, - con_id.get_target_node_id(), - con_id.get_target_thread(), - con_id.get_synapse_model_id(), - con_id.get_port() ); - - long target_node_id = getValue< long >( result_dict, names::target ); - double weight = getValue< double >( result_dict, names::weight ); - double delay = getValue< double >( result_dict, names::delay ); + conn.get_target_node_id(), + conn.get_target_thread(), + conn.get_synapse_model_id(), + conn.get_port() ); + const long target_node_id = getValue< long >( result_dict, names::target ); + const double weight = getValue< double >( result_dict, names::weight ); + const double delay = getValue< double >( result_dict, names::delay ); + const Layer< D >* const tgt_layer = dynamic_cast< Layer< D >* >( target_layer.get() ); + const long tnode_lid = tgt_layer->node_collection_->get_nc_index( target_node_id ); + assert( tnode_lid >= 0 ); // Print source, target, weight, delay, rports - out << source_node_id << ' ' << target_node_id << ' ' << weight << ' ' << delay; - - Layer< D >* tgt_layer = dynamic_cast< Layer< D >* >( target_layer.get() ); - - out << ' '; - const long tnode_lid = tgt_layer->node_collection_->get_lid( target_node_id ); - assert( tnode_lid >= 0 ); + out << source_node_id << ' ' << target_node_id << ' ' << weight << ' ' << delay << ' '; tgt_layer->compute_displacement( source_pos, tnode_lid ).print( out ); out << '\n'; } From eac9b31b565b4e7676e99467a44330be63d46e3c Mon Sep 17 00:00:00 2001 From: Hans Ekkehard Plesser Date: Fri, 21 Jun 2024 10:32:20 +0200 Subject: [PATCH 09/11] Sort connection dump row-wise to make test implementation-invariant --- .../pytests/test_spatial/test_mask_anchors_boundaries.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/testsuite/pytests/test_spatial/test_mask_anchors_boundaries.py b/testsuite/pytests/test_spatial/test_mask_anchors_boundaries.py index f6cde63f22..d2a984fcfe 100644 --- a/testsuite/pytests/test_spatial/test_mask_anchors_boundaries.py +++ b/testsuite/pytests/test_spatial/test_mask_anchors_boundaries.py @@ -139,7 +139,13 @@ def compare_layers_and_connections(use_free_mask, tmp_path, mask_params, edge_wr assert np.all(stored_src == src_layer_ref) assert np.all(stored_target == target_layer_ref) - assert np.all(stored_connections == connections_ref) + + # The order in which connections are written to file is implementation dependent. Therefore, we need to + # sort results and expectations here. We use lexsort to sort entire rows of the arrays. We need to + # transpose the array to provide it properly as keys to lexsort. + np.testing.assert_equal( + stored_connections[np.lexsort(stored_connections.T), :], connections_ref[np.lexsort(connections_ref.T), :] + ) @pytest.mark.parametrize( From 3c9c445320616be63f2023b1a97f1621c5004401 Mon Sep 17 00:00:00 2001 From: Hans Ekkehard Plesser Date: Fri, 21 Jun 2024 10:33:01 +0200 Subject: [PATCH 10/11] Correctly initialize loop helpers and modernize loop --- nestkernel/layer_impl.h | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/nestkernel/layer_impl.h b/nestkernel/layer_impl.h index 979764146d..e24f571d08 100644 --- a/nestkernel/layer_impl.h +++ b/nestkernel/layer_impl.h @@ -310,24 +310,22 @@ Layer< D >::dump_connections( std::ostream& out, AbstractLayerPTR target_layer, const Token& syn_model ) { - std::vector< std::pair< Position< D >, size_t > >* src_vec = get_global_positions_vector( node_collection ); - - // Dictionary with parameters for get_connections() + // Find all connections for given sources, targets and synapse model DictionaryDatum conn_filter( new Dictionary ); - def( conn_filter, names::synapse_model, syn_model ); - def( conn_filter, names::target, NodeCollectionDatum( target_layer->get_node_collection() ) ); def( conn_filter, names::source, NodeCollectionDatum( node_collection ) ); - + def( conn_filter, names::target, NodeCollectionDatum( target_layer->get_node_collection() ) ); + def( conn_filter, names::synapse_model, syn_model ); ArrayDatum connectome = kernel().connection_manager.get_connections( conn_filter ); - // Get variables for loop - size_t previous_source_node_id = getValue< ConnectionDatum >( connectome.get( 0 ) ).get_source_node_id(); - Position< D > source_pos = src_vec->begin()->first; + // Get positions of remote nodes + std::vector< std::pair< Position< D >, size_t > >* src_vec = get_global_positions_vector( node_collection ); - // Print information about all local connections for current source - for ( size_t i = 0; i < connectome.size(); ++i ) + // Iterate over connectome and write every connection, looking up source position only if source neuron changes + size_t previous_source_node_id = 0; // dummy initial value, cannot be node_id of any node + Position< D > source_pos; // dummy value + for ( const auto& entry : connectome ) { - ConnectionDatum conn = getValue< ConnectionDatum >( connectome.get( i ) ); + ConnectionDatum conn = getValue< ConnectionDatum >( entry ); const size_t source_node_id = conn.get_source_node_id(); // Search source_pos for source node only if it is a different node @@ -336,7 +334,8 @@ Layer< D >::dump_connections( std::ostream& out, const auto it = std::find_if( src_vec->begin(), src_vec->end(), [ source_node_id ]( const std::pair< Position< D >, size_t >& p ) { return p.second == source_node_id; } ); - assert( it != src_vec->end() ); + assert( it != src_vec->end() ); // internal error if node not found + source_pos = it->first; previous_source_node_id = source_node_id; } From cab8973bc6e9851b6143e1a43dcb573e25cc5deb Mon Sep 17 00:00:00 2001 From: Hans Ekkehard Plesser Date: Fri, 21 Jun 2024 12:06:48 +0200 Subject: [PATCH 11/11] Make further tests invariant against connection dump output order --- testsuite/pytests/test_spatial/test_weight_delay.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/testsuite/pytests/test_spatial/test_weight_delay.py b/testsuite/pytests/test_spatial/test_weight_delay.py index 3cd0a76180..f0569d80b0 100644 --- a/testsuite/pytests/test_spatial/test_weight_delay.py +++ b/testsuite/pytests/test_spatial/test_weight_delay.py @@ -118,8 +118,9 @@ def test_layer_connections_dump(tmp_path, expected_conn_dump, layer_type): fname = tmp_path / f"{layer_type}_layer_conns.txt" nest.DumpLayerConnections(src_layer, tgt_layer, "static_synapse", fname) + # We need to sort results to be invariant against implementation-dependent output order actual_conn_dump = fname.read_text().splitlines() - assert actual_conn_dump == expected_conn_dump + assert actual_conn_dump.sort() == expected_conn_dump.sort() @pytest.fixture(scope="module")