Skip to content
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

Unify synaptic recordable names for PSC models #502

Merged
merged 2 commits into from
Oct 6, 2016

Conversation

Silmathoron
Copy link
Member

@Silmathoron Silmathoron commented Oct 4, 2016

This is a fix for issue #501
The PR sets recordable names for synaptic currents as "I_syn_ex" or "I_syn_in" for:

  • amat2_psc_exp (added recordables)
  • hh_psc_alpha
  • iaf_psc_alpha
  • iaf_psc_exp

and removed "I_ex, "I_in", "input_currents_ex", "input_currents_in" from nest_names.

Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost ok, would just like to have a few empty lines and inline declarations.

@@ -350,6 +350,16 @@ class amat2_psc_exp : public Archiving_Node
{
return S_.V_th_v_;
}
double
Copy link
Contributor

@heplesser heplesser Oct 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert and empty line before the function definition. Also, I would suggest to add an inline.

{
return S_.I_syn_ex_;
}
double
Copy link
Contributor

@heplesser heplesser Oct 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert and empty line before the function definition. Also, I would suggest to add an inline.

@@ -310,12 +310,12 @@ class iaf_psc_alpha : public Archiving_Node
return V_.weighted_spikes_in_;
}
double
get_input_currents_ex_() const
get_I_syn_ex_() const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added empty line and inline.

@@ -310,12 +310,12 @@ class iaf_psc_alpha : public Archiving_Node
return V_.weighted_spikes_in_;
}
double
get_input_currents_ex_() const
get_I_syn_ex_() const
{
return S_.y1_ex_;
}
double
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added empty line and inline.

@@ -313,12 +313,12 @@ class iaf_psc_exp : public Archiving_Node
return V_.weighted_spikes_in_;
}
double
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added empty line and inline.

@@ -313,12 +313,12 @@ class iaf_psc_exp : public Archiving_Node
return V_.weighted_spikes_in_;
}
double
get_input_currents_ex_() const
get_I_syn_ex_() const
{
return S_.i_syn_ex_;
}
double
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added empty line and inline.

@heplesser
Copy link
Contributor

👍 from me. @jhnnsnk , would you take another quick look?

@Silmathoron Silmathoron mentioned this pull request Oct 5, 2016
@DimitriPlotnikov
Copy link

Reasonable unification. Also OK from my side. @heplesser since this can break somebody's code, should one notify malinglist?

@heplesser
Copy link
Contributor

@DimitriPlotnikov Good idea to announce!

@heplesser heplesser merged commit 8014dd0 into nest:master Oct 6, 2016
@Silmathoron Silmathoron deleted the unified_psc_name branch October 6, 2016 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants