Skip to content

Add eprop work to main tools#1368

Draft
andrewgait wants to merge 144 commits into
masterfrom
eprop_adaptive_update
Draft

Add eprop work to main tools#1368
andrewgait wants to merge 144 commits into
masterfrom
eprop_adaptive_update

Conversation

@andrewgait

@andrewgait andrewgait commented Jul 6, 2023

Copy link
Copy Markdown
Contributor

Incorporate eprop models into the main tools.

This implementation effectively sits alongside the current "standard" neuron models and does not impinge too much on those; the current sticking points (which could be worked on) are:

  • It may be necessary to further silo the eprop implementation away from the standard models (for example, creating a spike_processing_eprop.c file and/or a synapses_eprop.c file)
  • In a similar vein, the standard Spike Source Poisson C code takes advantage of the multicast packet with payload to deal with cases where multiple spikes may be sent in the same timestep by sending them as one packet with a payload to indicate the number of spikes. This needed to be removed in the current eprop implementation because the communication of the learning signal into the eprop model population requires the use of the multicast packet with payload functionality. We could plausibly have two separate spike source poisson executables to denote these two scenarios.

oliverrhodes and others added 30 commits September 19, 2019 12:34
…ng for poisson sources to be update-able from this code, all poisson stuff has been removed, neuron index 1 and 2 has V stored as diff variables, mean and entropy of values comes from average V over the recall period
…s are corrected, init corrected to include store_recall and eprop properly, test script adjusted to creat edge for poisson updating, still not tested as waiting for erbp plasticity components
…e, created separate synapses.c for sinusoid to process neuron array separately, altered print outs, couple bug fixes
@coveralls

coveralls commented Aug 8, 2023

Copy link
Copy Markdown

Coverage Status

coverage: 60.979% (-0.3%) from 61.316% when pulling fa3478a on eprop_adaptive_update into 0f8e5b7 on master.

@Bekkozhan

Copy link
Copy Markdown

Hi all!

I'm working with Andrew Davison at the Institute of Neuroscience Paris-Saclay on extending PyNN to support spike-based learning algorithms such as e-prop and EventProp across neuromorphic backends. I've been reviewing this PR as part of a SpiNNaker feasibility assessment. The implementation looks complete, with two known integration sticking points noted in the description. Is there still active interest in resolving those and getting this merged into the main tools?

Bek

@andrewgait

Copy link
Copy Markdown
Contributor Author

Hello @Bekkozhan - this was worked on a number of years ago, and I am not sure how easy it will be to update and merge into the main tools - I don't work on this project any more (though I keep an eye out for things that happen). I'm tagging @rowleya and @Christian-B who are still working on it to see if they have any opinions about whether this branch/PR is worth reviving.

@Bekkozhan

Copy link
Copy Markdown

Thank you @andrewgait - will wait for @rowleya and @Christian-B to comment on this.

@rowleya

rowleya commented Jun 1, 2026

Copy link
Copy Markdown
Member

I can say that I would be interested in having this in the main code for people to use. EProp is still an important learning algorithm so a working implementation directly on SpiNNaker would be very nice to have!

In terms of the sticking points:

  • I would need to take a more detailed look to understand what might need to be done regarding spike processing.
  • I looked at the use of payloads for the Poisson source elsewhere as I needed to use payloads for a different reason. I think that the memory-transfer mode of Poisson sources is probably common when high-rate Poisson sources are used e.g. in balanced random networks as a source of per-neuron independent noise, thus negating the need for this payload-mode to be useful.

There are clearly some conflicts at this point that need resolving also; hopefully this wouldn't be too hard to do!

@Bekkozhan

Copy link
Copy Markdown

Thank you @rowleya — great to hear there's interest in moving this forward. Happy to help with the conflict resolution if that would be useful.

@rowleya

rowleya commented Jun 1, 2026

Copy link
Copy Markdown
Member

Happy to help with the conflict resolution if that would be useful.

Yes, you are very welcome to look and see if you can resolve any of it! I may not have much time to look this week...

@Bekkozhan

Copy link
Copy Markdown

Hi Andrew (@rowleya) — following up on your comment, I've resolved 10 of the 15 conflicting files and pushed to a branch on my fork: Bekkozhan/sPyNNaker:eprop-master-merge.

Resolved (no functional changes):

  • extra_models/__init__.py, builds/__init__.py, neuron_models/__init__.py, threshold_types/__init__.py — union of eprop and master exports
  • abstract_standard_neuron_component.py — kept eprop's uses_eprop property + master's _convert() utility
  • neuron_impl_standard.py — kept eprop's neuron_model property + master's type annotation
  • abstract_connector.py — kept eprop's array handling + master's exception helpers
  • population_machine_synapses.py, population_machine_vertex.py, population_vertex.py — eprop logic preserved, updated to master's _pop_vertex naming throughout

Five files left at the eprop version — three decisions needed:

  1. Signed weights (synapse_dynamics_static.py, synapse_dynamics_stdp.py, synapses.c) — eprop needs signed weights; master uses numpy.abs. Should signed weights be supported in the merged version?
  2. Poisson payload (spike_source_poisson.c, Makefile) — your suggested two-executable approach makes sense; would you confirm the split point (spike_processing vs synapses layer)?
  3. Ring buffer power forcing — in get_ring_buffer_shifts, the eprop branch forced all powers to 2 with a TODO: check this EPROP note from andrewgait. I dropped this when adopting master's version. Does eprop require a fixed ring buffer power for correct weight scaling?

Happy to implement whatever you decide on points 1–3.

@Bekkozhan

Copy link
Copy Markdown

Update: following your emails, 3 more files are resolved — eprop-master-merge is now at 13/15.

  • Signed weights: confirmed weights cross zero during training (symmetric w_min=-max_weight, w_max=+max_weight in the e-prop configuration, no sign-preservation in the C update). Kept eprop's signed int16 encoding in synapse_dynamics_static.py and synapse_dynamics_stdp.py.
  • Poisson payload: turns out the payload-as-spike-count code was already disabled in the eprop branch — no change needed there. The one actual conflict in spike_source_poisson.c was an unrelated REAL/UREAL type mismatch for rate_per_tick; took master's UREAL version, which matches the function's own signature.
  • Ring buffer power forcing: already resolved in the first batch — population_vertex.py uses master's scaling without forcing, matching your preference.

Remaining: Makefile and synapses.c, kept at eprop version pending the C-level isolation discussion. Happy to file issues for both per your suggestion.

@rowleya

rowleya commented Jun 17, 2026

Copy link
Copy Markdown
Member

OK, so I had a bit of a discussion this morning with developers and have some more ideas and suggestions:

Signed weights: confirmed weights cross zero during training (symmetric w_min=-max_weight, w_max=+max_weight in the e-prop configuration, no sign-preservation in the C update). Kept eprop's signed int16 encoding in synapse_dynamics_static.py and synapse_dynamics_stdp.py.

The only thing to check here is that the sign passes through all the code, including the ring buffers themselves. In general there is no issue with signed values, so long as the handling is done correctly. I might then suggest that if the code no longer performs an "abs" on the weights (or if it does, remove that), then a test where there are synapses with mixed positive and negative weights in a single connection might be worth testing. The only issues I could potentially predict would be on the weight scaling, since this now needs to make sure it accounts for the sign.

Poisson payload: turns out the payload-as-spike-count code was already disabled in the eprop branch — no change needed there. The one actual conflict in spike_source_poisson.c was an unrelated REAL/UREAL type mismatch for rate_per_tick; took master's UREAL version, which matches the function's own signature.

That all sounds fine in that case!

Ring buffer power forcing: already resolved in the first batch — population_vertex.py uses master's scaling without forcing, matching your preference.

Great!

Remaining: Makefile and synapses.c, kept at eprop version pending the C-level isolation discussion. Happy to file issues for both per your suggestion.

OK, on synapses.c, I just checked and I see that the saturation is disabled. This is a critical issue I think, since if you saturate at this point, it will go very wrong I think! This was designed to be a very fast check, because adding synapses is a very common operation. I guess the issue here is the signed synapses again... how do you do a saturation check on a signed integer which might saturate in either direction? I will have a think about this and see if there is a general and fast solution!

On the Makefile, it looks like it just needs merging together; we have added some stuff into master since this PR was created.

@Bekkozhan

Copy link
Copy Markdown

@rowleya thanks for looking into this — pushed the Makefile merge now (14/15 resolved).
On the saturation question: the disable is in the same commit that introduced signed weights (532a6da, "add signed weights functionality", Oct 2019) — it changes weight from uint32_t to int32_t and comments out the saturation check in the same diff.
I'll leave synapses.c at the eprop version for now.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants