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

Implement e-prop plasticity #2867

Merged
merged 416 commits into from
Feb 28, 2024
Merged

Implement e-prop plasticity #2867

merged 416 commits into from
Feb 28, 2024

Conversation

akorgor
Copy link
Contributor

@akorgor akorgor commented Aug 7, 2023

This PR ports the eligibility propagation (e-prop) plasticity mechanism in supervised tasks published by Bellec et al. (2020) from TensorFlow to NEST.

The PR includes:

  • the neuron models eprop_iaf_psc_delta, eprop_iaf_psc_delta_adapt, and eprop_readout
  • the synapse model eprop_synapse
  • the connection model eprop_learning_signal_connection
  • the specific archiving node eprop_archiving_node to archive the history of variables required for computing the weight update
  • the unit test test_eprop_plasticity.py
  • the two examples scripts eprop_supervised_regression.py and eprop_supervised_classification.py

A manuscript on the details of this implementation, Korcsak-Gorzo, Stapmanns, and Espinoza Valverde et al., is in preparation.

Bellec, G., Scherr, F., Subramoney, A., Hajek, E., Salaj, D., Legenstein, R., & Maass, W. (2020). A solution to the learning dilemma for recurrent networks of spiking neurons. Nature communications, 11(1), 3625.

Korcsak-Gorzo A, Stapmanns J, Espinoza Valverde JA, Dahmen D, van Albada SJ, Bolten M, Diesmann M. Event-based implementation of eligibility propagation (in preparation)

Co-authored-by: Jonas Stapmanns [email protected]
Co-authored-by: JesusEV [email protected]

@terhorstd terhorstd added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Aug 14, 2023
@jessica-mitchell
Copy link
Contributor

@akorgor @JesusEV @jstapmanns FYI Here is the build output of the documentation from Read the docs for the PR. I've also created a PR against the eprop_feature branch in jstapmanns fork.

Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

Thanks just a few minor typos and formatting

models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_synapse.h Outdated Show resolved Hide resolved
models/eprop_synapse.h Outdated Show resolved Hide resolved
models/eprop_synapse.h Outdated Show resolved Hide resolved
nestkernel/archiving_node.h Outdated Show resolved Hide resolved
testsuite/pytests/test_eprop_plasticity.py Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.h Outdated Show resolved Hide resolved
nestkernel/node.cpp Show resolved Hide resolved
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.

Thanks a lot for this PR! @akorgor, @JesusEV and @heplesser reviewed most of this code today, except the new neuron models. It looks already pretty good, some cleanup to do.

pynest/nest/__init__.py Outdated Show resolved Hide resolved
pynest/nest/__init__.py Outdated Show resolved Hide resolved
pynest/nest/__init__.py Outdated Show resolved Hide resolved
testsuite/pytests/test_eprop_plasticity.py Outdated Show resolved Hide resolved
pynest/examples/eprop_plasticity/README.rst Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
models/eprop_synapse.h Outdated Show resolved Hide resolved
Copy link
Contributor Author

@akorgor akorgor left a comment

Choose a reason for hiding this comment

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

Processed part of the requested changes.

nestkernel/exceptions.cpp Outdated Show resolved Hide resolved
nestkernel/exceptions.h Outdated Show resolved Hide resolved
nestkernel/histentry.h Outdated Show resolved Hide resolved
nestkernel/nest_names.h Outdated Show resolved Hide resolved
nestkernel/histentry.h Outdated Show resolved Hide resolved
nestkernel/secondary_event.h Outdated Show resolved Hide resolved
nestkernel/secondary_event.h Outdated Show resolved Hide resolved
nestkernel/node.h Outdated Show resolved Hide resolved
nestkernel/archiving_node.h Outdated Show resolved Hide resolved
nestkernel/nest_names.h Show resolved Hide resolved
@akorgor akorgor requested review from jessica-mitchell and removed request for janskaar and JanVogelsang October 4, 2023 08:29
Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

looks really good, just have a few minor changes :)

models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_synapse_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_synapse_iaf_psc_delta_adapt.h Outdated Show resolved Hide resolved
models/eprop_synapse_readout.h Outdated Show resolved Hide resolved
pynest/examples/eprop_plasticity/README.rst Show resolved Hide resolved
Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this; still have a one typo but I will approve

models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
models/eprop_iaf_psc_delta.h Outdated Show resolved Hide resolved
@terhorstd
Copy link
Contributor

terhorstd commented Oct 6, 2023

Small hint: You can type pre-commit install once in the base of your repository to never have flake8, pycodestyle, … fail again. The first time you may want to additionally run pre-commit run --all-files to check the broken ones from earlier commits, too.

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.

This is a preliminary review of the files surrounding the actual neuron and synapse models. I will get back to those soon, but want to provide some feedback already now.

nestkernel/simulation_manager.h Show resolved Hide resolved
pynest/examples/eprop_plasticity/README.rst Outdated Show resolved Hide resolved
pynest/nest/__init__.py Outdated Show resolved Hide resolved
pynest/nest/__init__.py Outdated Show resolved Hide resolved
models/eprop_readout.cpp Outdated Show resolved Hide resolved
testsuite/pytests/test_eprop_plasticity.py Outdated Show resolved Hide resolved
testsuite/pytests/test_eprop_plasticity.py Outdated Show resolved Hide resolved
testsuite/pytests/test_eprop_plasticity.py Outdated Show resolved Hide resolved
testsuite/pytests/test_eprop_plasticity.py Outdated Show resolved Hide resolved
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.

@akorgor @JesusEV I have now reviewed everything in nestkernel, with a number of suggestions for further improving EpropArchivingNode. I will work on the material in models tomorrow. Some of my comments are just in a single place, but apply generally.

One more general aspect: You use long throughout, but I think in almost all places you could never have negative values. Our practice now is to use size_t in those cases.

nestkernel/eprop_archiving_node.h Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.h Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.h Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.h Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.h Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
nestkernel/eprop_archiving_node.cpp Outdated Show resolved Hide resolved
akorgor and others added 12 commits February 15, 2024 07:23
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
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.

@akorgor @JesusEV Great work, in my view this is now ready :)!

@heplesser heplesser removed the request for review from terhorstd February 27, 2024 16:07
@heplesser
Copy link
Contributor

@akorgor All reviewers have approved now, but the pylint test fails because Github recently started using a newer pylint version. We just merged a fix for this into master, so if you could merge master into your branch once more, this PR should be ready to be merged :).

@heplesser heplesser merged commit ff23f32 into nest:master Feb 28, 2024
24 checks passed
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: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants