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

Register only spikes actually sent with weight_recorder #3044

Merged
merged 5 commits into from
Dec 11, 2023

Conversation

heplesser
Copy link
Contributor

Until now, spikes would be registered with a weight_recorder even if the spike was not transmitted to the target, e.g., by a bernoulli_synapse. This PR changes all Connection<T>::send() methods to return true if they actually transmit a spike to the target node (call e();) and false otherwise.

The PR also simplifies the code in the bernoulli_synapse. Since this synapse model can only be used between normal nodes (not to/from devices), it will only receive events with multiplicity 1, allowing for significant simplification.

@JesusEV @terhorstd I'd appreciate if you could fast-track this PR since e-prop work depends on it.

…on<T>::send() must return whether they sent a spike or not.
@heplesser heplesser added T: Bug Wrong statements in the code or documentation S: High Should be handled next I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Dec 11, 2023
@heplesser heplesser added this to In progress in Kernel via automation Dec 11, 2023
Copy link
Contributor

@JesusEV JesusEV left a comment

Choose a reason for hiding this comment

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

I applied these changes to a copy of the latest eprop_feature and applied the corresponding changes to eprop_synapse.h (returning false if the spike is to be ignored) and learning_signal_connection.h.

All verifications were successful.

By themselves, and all in all, the changes seem very appropriate

heplesser and others added 2 commits December 11, 2023 15:20
Co-authored-by: Dennis Terhorst <terhorstd@users.noreply.github.com>
Copy link
Contributor

@terhorstd terhorstd left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@heplesser heplesser merged commit 1ab8c5d into nest:master Dec 11, 2023
24 checks passed
Kernel automation moved this from In progress to Done Dec 11, 2023
@heplesser heplesser deleted the fix-wr-call branch April 24, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Bug Wrong statements in the code or documentation
Projects
Kernel
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants