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

Correct input handling for aeif_cond_beta_multisynapse #510 and Update aeif_cond_beta_multisynapse to new aeif numerics approach #514 #516

Merged
merged 25 commits into from Oct 26, 2016
Commits
Jump to file or symbol
Failed to load files and symbols.
+5 −4
Diff settings

Always

Just for now

Viewing a subset of changes. View all

minor fixes to formatting

  • Loading branch information...
golosio committed Oct 14, 2016
commit f9f7a19f06d1cae3654ebf419c4fe77491298f5b
@@ -165,13 +165,14 @@ aeif_cond_beta_multisynapse::Parameters_::set( const DictionaryDatum& d )
updateValue< std::vector< double > >( d, names::taus_decay, taus_decay );
if ( Erev_flag || taur_flag || taud_flag )

This comment has been minimized.

@heplesser

heplesser Oct 15, 2016

Contributor

I think it should be possible to simplify this some more:

// n_receptors() == E_rev.size()
if ( n_receptors() != taus_rise.size() || n_receptors() != taus_decay.size() )
{
  throw BadProperty("E_rev, taus_raus and taus_decays must have the same size.");
}

Now we know all is consistent I would then check n_receptors() != old_n_receptors && has_connections and n_receptors > 0. Then the check that the taus > 0 and tau_rise < tau_decay. That should maybe be protected by if ( taur_flag || taud_flag ) since we have loops here and would want to avoid running through them if the taus have not been changed.

@heplesser

heplesser Oct 15, 2016

Contributor

I think it should be possible to simplify this some more:

// n_receptors() == E_rev.size()
if ( n_receptors() != taus_rise.size() || n_receptors() != taus_decay.size() )
{
  throw BadProperty("E_rev, taus_raus and taus_decays must have the same size.");
}

Now we know all is consistent I would then check n_receptors() != old_n_receptors && has_connections and n_receptors > 0. Then the check that the taus > 0 and tau_rise < tau_decay. That should maybe be protected by if ( taur_flag || taud_flag ) since we have loops here and would want to avoid running through them if the taus have not been changed.

This comment has been minimized.

@golosio

golosio Oct 15, 2016

Contributor

I agree that the code can be made more compact if we simply want to check consistency. However my idea is that we should not only check for consistency, but also give the user a feedback that suggests him how he should modify his command. For instance, if the current number of receptor ports is 4 (or if they have not yet been initialized so the number is 0 or 1) and the user types a command as:
nest.SetStatus(neuron, {'taus_decay':[50.0,20.0,20.0]})
I think a message as:
"If the number of receptor ports is changed, all three arrays E_rev, taus_rise and taus_decay must be provided."
is more helpful to the user than:
"E_rev, taus_rise and taus_decay must have the same size."
because with the latter message it may not be obvious to all users that the problem can be solved by providing all three arrays as arguments.

@golosio

golosio Oct 15, 2016

Contributor

I agree that the code can be made more compact if we simply want to check consistency. However my idea is that we should not only check for consistency, but also give the user a feedback that suggests him how he should modify his command. For instance, if the current number of receptor ports is 4 (or if they have not yet been initialized so the number is 0 or 1) and the user types a command as:
nest.SetStatus(neuron, {'taus_decay':[50.0,20.0,20.0]})
I think a message as:
"If the number of receptor ports is changed, all three arrays E_rev, taus_rise and taus_decay must be provided."
is more helpful to the user than:
"E_rev, taus_rise and taus_decay must have the same size."
because with the latter message it may not be obvious to all users that the problem can be solved by providing all three arrays as arguments.

{ // receptor arrays have been modified
if ( ( E_rev.size() != old_n_receptors || taus_rise.size() !=
old_n_receptors || taus_decay.size() != old_n_receptors ) &&
( !Erev_flag || !taur_flag || !taud_flag ) )
if ( ( E_rev.size() != old_n_receptors
|| taus_rise.size() != old_n_receptors
|| taus_decay.size() != old_n_receptors )
&& ( !Erev_flag || !taur_flag || !taud_flag ) )
{
throw BadProperty(
"If the number of receptor ports is changed, all three arrays "
"E_rev, taus_rise and taus_decay must be provided.");
"E_rev, taus_rise and taus_decay must be provided." );
}
if ( ( E_rev.size() != taus_rise.size() )
|| ( E_rev.size() != taus_decay.size() ) )
ProTip! Use n and p to navigate between commits in a pull request.