Step 6 - Fused event generation and postsynaptic spike-like events#609
Merged
neworderofjamie merged 119 commits intogenn_5from Jan 31, 2024
Merged
Step 6 - Fused event generation and postsynaptic spike-like events#609neworderofjamie merged 119 commits intogenn_5from
neworderofjamie merged 119 commits intogenn_5from
Conversation
* Because ``NeuronGroup`` will merge spike conditions from incoming and outgoing synapses, need to make fusing-related ``SynapseGroup`` figure out whether they're pre or postsynaptic based on NeuronGroup. As a bonus, this reduced a lot of WUM duplicate code * Started adding data structures for spikes to SynapseGroup and NeuronGroup
…ough all the related hashing functions!
…et getting also needs to take neurongroup
…n in hierarchical merged groups!
…nt to update spike data structures
…ion, the fused targets
…to ``SynapseGroup``" This reverts commit 3bf3139. # Conflicts: # src/genn/genn/code_generator/neuronUpdateGroupMerged.cc
tnowotny
reviewed
Jan 8, 2024
Member
tnowotny
left a comment
tnowotny
left a comment
There was a problem hiding this comment.
Sounds like something that should be sorted but could be endlessly tricky. Are you confident you have covered all nasty confusions?
Merged
Contributor
Author
|
It is definitely tricky - I've been fixing bugs in the Final PR (#613) based on new tests/STDP models. However, I think it's the right thing to do and necessary as it lays the groundwork both for learning convolutions efficiently and more advanced event-driven three-factor learning. |
tnowotny
approved these changes
Jan 31, 2024
Member
tnowotny
left a comment
tnowotny
left a comment
There was a problem hiding this comment.
Approved though detailed code review at this scale is beyond what I can do.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One feature that has been notably missing from GeNN is postsynaptic spike-like events. These would be very useful for implementing all kinds of event-based learning rules e.g. to add per-MBON event-based dopamine signals to a mushroom body model. However, the spike-like event system was a bit delicate (see #379 and #594) and definitely couldn't take any more complexity!
This PR adds in this functionality and lays the ground-work for enabling the "spike-splitting" optimisation described in #484 which would hugely improve performance of convolutions. Essentially what this PR does is:
spkandspkCntfrom being per-neuron group to per-fused synapse group likeinSynetc if the appropriate option is enabled (for spikes it always is), reusing the same mechanism used for Merging of compatible postsynaptic models #201 and Merge pre and postsynaptic update #461 along with the more elegant merged group implementation from Step 1 - Transpiler #595.However, as ever, the devil is in the details and fusing these synapse groups is rather trickier than e.g. a postsynaptic update because, the neuron code doesn't care whether spike/spike-event thresholds are presynaptic or postsynaptic and should fuse them regardless. However this means that one synapse groups presynaptic end might be fused to another's postsynaptic end if they correspond to the same neuron group. The bugs this caused have been...fun...hence the additional logging functionality I've added to Runtime which should now tell you everything reading the source of runner.cc would have 😄