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

Changes to fix the hard coded decay of synaptic elements #215

Merged
merged 3 commits into from Feb 1, 2016

Conversation

@sdiazpier
Copy link
Contributor

@sdiazpier sdiazpier commented Jan 29, 2016

This pull request addresses issue #214 , now allowing the user to set the amount using the tau_vacant variable in the synaptic elements class.

@@ -47,7 +47,7 @@ nest::SynapticElement::SynapticElement()
, z_connected_( 0 )
, continuous_( true )
, growth_rate_( 1.0 )
, tau_vacant_( 10.0 )
, tau_vacant_( 0.1 )

This comment has been minimized.

@heplesser

heplesser Jan 31, 2016
Contributor

@sdiazpier I am a bit surprised by the change of the default value of tau_vacant_ from 10.0 to 0.1. This probably has no consequence, since the value was not used anywhere in the past, but could you just confirm that the change is intentional?

This comment has been minimized.

@sdiazpier

sdiazpier Feb 1, 2016
Author Contributor

Hi dear @heplesser, yes you are right, the change is intentional. This value was unused, but it was part of the original design. This change was done in order to adapt the value to the current implementation of the decay.

@@ -401,7 +401,7 @@ nest::Archiving_Node::decay_synaptic_elements_vacant( double_t p )
{
z = ( it->second ).get_z();
z_vacant = ( it->second ).get_z_vacant();
it->second.set_z( z - ( z_vacant * p ) );
it->second.set_z( z - ( z_vacant * it->second.get_tau_vacant() ) );
}

This comment has been minimized.

@heplesser

heplesser Jan 31, 2016
Contributor

@sdiazpier All quantities entering the operation in this loop are members of the SynapticElement object it->second. So I'd suggest to just call here it->second.update_z(), which should then look something like

{ z_ -= get_z_vacant() * tau_vacant_; }

Or have I overlooked something?

@heplesser
Copy link
Contributor

@heplesser heplesser commented Jan 31, 2016

I think @jakobj would be a good second reviewer.

@jakobj
Copy link
Contributor

@jakobj jakobj commented Feb 1, 2016

i agree with @heplesser's suggestion to introduce the update_z() function in order to avoid those getters, but otherwise i'm happy with the changes.

sdiazpier added 2 commits Feb 1, 2016
…update is handled directly inside the SynapticElements class
@sdiazpier
Copy link
Contributor Author

@sdiazpier sdiazpier commented Feb 1, 2016

Dear @jakobj and @heplesser, thank you for your comments. I have done and submitted the changes you suggested. Please let me know if anything else is missing.

@jakobj
Copy link
Contributor

@jakobj jakobj commented Feb 1, 2016

looks good to me 👍

@heplesser
Copy link
Contributor

@heplesser heplesser commented Feb 1, 2016

Thank you! 👍

heplesser added a commit that referenced this pull request Feb 1, 2016
Changes to fix the hard coded decay of synaptic elements
@heplesser heplesser merged commit 580af19 into nest:master Feb 1, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.