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

Fixed bug in delivery of secondary events #711

Merged
merged 1 commit into from Apr 18, 2017

Conversation

Projects
None yet
3 participants
@janhahne
Contributor

janhahne commented Apr 12, 2017

This fixes #708. The break statement was accidentally not removed with the original PR #175 which should already have fixed this issue.

Originally there was a one-to-one mapping between syn_id and SecondaryEvent classes. Meaning only gap_junctions used GapJunctionEvents. The property get_syn_id() was replaced by supports_syn_id with #175 to allow the use of CopyModel and due to the introduction of labeled synapses, as both gap_junction and gap_junction_lbl of course use the GapJunctionEvent.

The here removed break statement was (in the days of the one-to-one mapping) used to improve the performance and then by mistake not removed when #175 was introduced.

@heplesser Do you remember the original PR? If yes, could you have a quick look?

@jakobj

This comment has been minimized.

Show comment
Hide comment
@jakobj

jakobj Apr 18, 2017

Contributor

Seems good, but could you quickly remind me why after this change, events are not delivered to more than one homogeneous connector? as far as I understand, after the copy gap events support multiple syn_id. so I would expect a gap event targeting neuron1 to also be delivered to connections targeting neuron2, precicely because we are only checking for "supports" not equality.

Contributor

jakobj commented Apr 18, 2017

Seems good, but could you quickly remind me why after this change, events are not delivered to more than one homogeneous connector? as far as I understand, after the copy gap events support multiple syn_id. so I would expect a gap event targeting neuron1 to also be delivered to connections targeting neuron2, precicely because we are only checking for "supports" not equality.

@janhahne

This comment has been minimized.

Show comment
Hide comment
@janhahne

janhahne Apr 18, 2017

Contributor

@jakobj With this change events are delivered to more than one (of course still only fitting) connector, that is the fix. Or am I understanding you wrong?

Contributor

janhahne commented Apr 18, 2017

@jakobj With this change events are delivered to more than one (of course still only fitting) connector, that is the fix. Or am I understanding you wrong?

@jakobj

This comment has been minimized.

Show comment
Hide comment
@jakobj

jakobj Apr 18, 2017

Contributor

Hmm, I am bit confused. I guess my question is, where is it assured that events via syn0 are only delivered to neuron_out1 and from syn1 only to neuron_out2 if both events support the same syn_ids?

Contributor

jakobj commented Apr 18, 2017

Hmm, I am bit confused. I guess my question is, where is it assured that events via syn0 are only delivered to neuron_out1 and from syn1 only to neuron_out2 if both events support the same syn_ids?

@jakobj

This comment has been minimized.

Show comment
Hide comment
@jakobj

jakobj Apr 18, 2017

Contributor

nevermind. I think I've got my mind untangled now. :) so 👍 from me

Contributor

jakobj commented Apr 18, 2017

nevermind. I think I've got my mind untangled now. :) so 👍 from me

@jakobj

jakobj approved these changes Apr 18, 2017

@heplesser heplesser merged commit efd79cb into nest:master Apr 18, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@janhahne janhahne deleted the janhahne:fix_708 branch Apr 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment