Revise sp_manager::global_shuffle

[heplesser]

Currently, the behavior of sp_manager::global_shuffle() does not agree with the comment describing the method:

• The comment states that the method "shuffles the first n elements of the vector", implying that the elements from n+1 on remain in the vector in original order
• The method randomly picks n elements from the entire vector without replacement and then replaces the content of the vector with these first n elements, dropping the rest

The way the method is used (eg here

nest-simulator/nestkernel/sp_manager.cpp

Lines 439 to 440 in 5af6664

	global_shuffle( pre_id_rnd, post_id_rnd.size() );
	pre_id_rnd.resize( post_id_rnd.size() );

) indicates that the actual behavior is what is intended (but the resize() after calling the method is then not necessary).

Comment and code need to be brought into agreement. One might also consider a more informative method name, including the "chopping" part. The implementation should be reviewed as well. It should be able to work in-place without need of the "erase" function (costly). It might make sense to use suitable C++11 functions, e.g. https://www.cplusplus.com/reference/algorithm/random_shuffle/. Finally, variable names in the method are very uninformative and should be revised.

[github-actions]

Issue automatically marked stale!

[heplesser]

@sdiazpier @pnbabu Ping!

[pradkrish]

Hello, Is this issue still open? I am happy to give it a go.

[heplesser]

Yes, the issue appears still to be open.

[github-actions]

Issue automatically marked stale!