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

Moved vacant sp elements sanity check to decay function #827

Merged
merged 3 commits into from Oct 13, 2017

Conversation

Projects
None yet
4 participants
@sdiazpier
Copy link
Contributor

sdiazpier commented Sep 19, 2017

This PR follows up on #818. The check on the vacant synaptic elements is moved to the decay function of the synaptic elements to ensure that the deletion of elements still takes place correctly by the sp_manager.

I suggest @sanjayankur31 as reviewer for this.

@sanjayankur31

This comment has been minimized.

Copy link
Contributor

sanjayankur31 commented Sep 19, 2017

👍

Looks good! That should work much better! Thanks @sdiazpier

sdiazpier added some commits Sep 19, 2017

@heplesser

This comment has been minimized.

Copy link
Contributor

heplesser commented Oct 12, 2017

@sanjayankur31 @sdiazpier After this change, Archiving_Node::get_synaptic_elements_vacant() may return a negative value. Is that sensible?

@sanjayankur31 Please use the "Review changes" button in the "Files changed" tab to approve or disapprove---searching for 👍 in discussions can be tedious.

@sanjayankur31

This comment has been minimized.

Copy link
Contributor

sanjayankur31 commented Oct 12, 2017

@heplesser - the way structural plasticity is implemented, ArchivingNode::get_synaptic_elements_vacant() < 0 implies that connections need to be deleted. It's used in SPManager::get_synaptic_elements(). (I hadn't realised this either which is why my earlier fix in #818 was incomplete).

I'll use the review tools. Sorry about that!

@heplesser heplesser added this to the NEST 2.14.0 milestone Oct 13, 2017

@heplesser heplesser merged commit ab2b396 into nest:master Oct 13, 2017

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
You can’t perform that action at this time.