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

Access to postsynaptic variables with heterogeneous delay #629

Merged
merged 44 commits into from
Jul 8, 2024

Conversation

neworderofjamie
Copy link
Contributor

@neworderofjamie neworderofjamie commented Jun 29, 2024

This PR enables you to access postsynaptic neuron variable references and postsynaptic weight update model variables in synapse code (pre, post spike/spike-like event triggered or synapse dynamics) using an array subscript expression to specify a potentially heterogeneous delay slot e.g. x_post[d]. This interacts with the existing delay system as follows:

  • If any postsynaptic neuron variable references are made with new [] syntax
    • Number of postsynaptic neuron group delay slots needs to be made at least as large as max dendritic delay
    • Target neuron variables need to be marked as queued using the existing mechanism so they are allocated + initialised with delay slots
  • If any postsynaptic weight update model variables are accessed with new [] syntax
    • Again, the number of postsynaptic neuron group delay slots needs to be made at least as large as max dendritic delay
    • With the old homogeneous delays, if synapse group was delayed, all weight update model pre and postsynaptic variables were delayed. However, now, some postsynaptic weight update model variables might be delayed and some not. To address this I've added a very similar mechanism to the neuron variable queuing to synapse group which has repercussions for fusing etc etc

One side effect of this change is that neuron groups might now have some variables which require queueing but no axonal/backpropagation delays applied to their spikes. This resulted in unnecessarily complex code being generated for spike handling so I have extended the existing tracking of which variables are queued to also track whether spikes or spike events are queued.

The handling of the actual [d] syntax is implemented by adding x_post to the type system as a special sort of function (with signature scalar x_post(int index) (assuming the type of x_post is scalar). These functions are tagged with the Type::FunctionFlags::ARRAY_SUBSCRIPT_OVERRIDE flag. The type checker and pretty printer then treat, [] as a special sort of function call which enables all the existing substitution magic used to stick code behind functions to be used.

In the process of making this change, I tidied up a few things:

  • Added an error if addToPostDelay is called or the new [] syntax is used in synapse code but no maximum number of dendritic delay steps has been specified.
  • Types in the type system don't really need an enum of qualifiers - the only qualifier that's ever going to be supported is const so I have simplified a bunch of code by replacing this with a simple bool. Conversely, function types do need some more flags so replaced variadic boolean with some flags (now including Type::FunctionFlags::ARRAY_SUBSCRIPT_OVERRIDE )
  • Because of type promotion, it doesn't really matter but the types of addToPre functions etc actually vary depending on what type scalar is so constants like Type::AddToPre have been replaced by functions like Type::getAddToPre(Type::Float) which generate the right function type.
  • The syntax for EnvironmentLocalVarCache was unnecessarily complex - the adaptor type being passed in already provides the required information about delay an 99% of the time this is what determines whether variables should be copied between delay slots every timestep.
  • Finally got annoyed enough to update some warnings about deprecated kwargs in the tests

@neworderofjamie neworderofjamie added this to the GeNN 5.1.0 milestone Jun 29, 2024
Copy link

codecov bot commented Jun 29, 2024

Codecov Report

Attention: Patch coverage is 95.57110% with 19 lines in your changes missing coverage. Please review.

Project coverage is 88.33%. Comparing base (78dc786) to head (0f4fafb).
Report is 3 commits behind head on master.

Files Patch % Lines
...enn/genn/code_generator/neuronUpdateGroupMerged.cc 88.88% 4 Missing ⚠️
...nn/genn/code_generator/synapseUpdateGroupMerged.cc 93.75% 4 Missing ⚠️
src/genn/genn/transpiler/prettyPrinter.cc 88.00% 3 Missing ⚠️
include/genn/genn/type.h 94.11% 2 Missing ⚠️
src/genn/genn/code_generator/backendSIMT.cc 91.66% 2 Missing ⚠️
...clude/genn/genn/customConnectivityUpdateInternal.h 87.50% 1 Missing ⚠️
...nn/code_generator/presynapticUpdateStrategySIMT.cc 97.22% 1 Missing ⚠️
src/genn/genn/synapseGroup.cc 97.29% 1 Missing ⚠️
src/genn/genn/transpiler/typeChecker.cc 96.42% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #629      +/-   ##
==========================================
+ Coverage   88.27%   88.33%   +0.06%     
==========================================
  Files         106      106              
  Lines       14546    14675     +129     
==========================================
+ Hits        12840    12963     +123     
- Misses       1706     1712       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@neworderofjamie neworderofjamie marked this pull request as ready for review July 2, 2024 17:07
@neworderofjamie neworderofjamie requested a review from tnowotny July 2, 2024 17:09
…g to be used for standard quantifiers replaced with simple isConst boolean
…c delay to correctly set max dendritic delay
…th or without delayed [] syntax and unit test
* Helper in weight update init to determine if variable is accessed with a heterogeneous delay in synapse code
* Added methods to relevant adaptors to call helper
* Use addVarRefsHet in SynapseGroupMerged to generate appropriate index code
Copy link
Member

@tnowotny tnowotny left a comment

Choose a reason for hiding this comment

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

I think it all makes sense. For a moment I thought the test for the continuous reverse delay synapse was a bit weak in that it did not test that reverse add could happen at different times and not test from which synapse/post neuron the reverse input was coming, but on reflection, it seems unlikely one can pass the current test if there was a bug.

@neworderofjamie neworderofjamie merged commit 647cf0f into master Jul 8, 2024
3 checks passed
@neworderofjamie neworderofjamie deleted the dendritic_delay_vars branch July 8, 2024 11:26
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.

2 participants