New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make Connector properties and growth factor configurable #850
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jougs Thanks for picking this up! I just have two tiny suggestions. Once those are addressed, we can merge, since @mschmidt87 approved the predecessor #507.
nestkernel/nest_names.cpp
Outdated
@@ -238,6 +238,7 @@ const Name index_map( "index_map" ); | |||
const Name individual_spike_trains( "individual_spike_trains" ); | |||
const Name inh_conductance( "inh_conductance" ); | |||
const Name init_flag( "init_flag" ); | |||
const Name init_connector_capacity( "init_connector_capacity" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep alphabetical order, this one should come before init_flag
.
nestkernel/connection_manager.cpp
Outdated
@@ -134,7 +134,7 @@ void | |||
nest::ConnectionManager::set_status( const DictionaryDatum& d ) | |||
{ | |||
long init_cap = init_conn_capacity_; | |||
if ( updateValue< long >( d, "init_connector_capacity", init_cap ) ) | |||
if ( updateValue< long >( d, names::init_connector_capacity, init_cap ) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would change this name to initial_connector_capacity
. Just init_...
evokes too strong associations to the verb "initialize" and may therefore be confusing.
Merging now, since @mschmidt87 had approved precursor PR #507 previously and this PR added only minor fixed. Thanks, @jougs and @alexeyshusharin! |
This PR replaces #507. For explanations and discussion see the comments there.