brian-team / brian2genn

Brian 2 frontend to the GeNN simulator
http://brian2genn.readthedocs.io/
GNU General Public License v2.0
46 stars 16 forks source link

Support for post/pre-synaptic variables in on_post/pre statements #99

Open kjohnsen opened 4 years ago

kjohnsen commented 4 years ago

I'm trying to run the Clopath et al. 2010 Brian example using brian2genn, but apparently referencing post-synaptic variables in on_post statements is not supported.

Is there a workaround for this? Or would it be possible to implement this? In any case it would be nice to see this in the unsupported features section of the documentation.

tnowotny commented 4 years ago

As I recall it, it is modifying post-synaptic variables in the on_post code which is not supported. However, I am not sure how much this restriction is historic. If I am not mistaken, the learning kernel (where the "on_post" code goes in GeNN) is parallelised along the post-synaptic spiking neurons and hence there would not be a race condition/ unpredictable outcome risk if one would allow "+=" etc modifications of post-synaptic variables (what do you think @neworderofjamie ?). @mstimberg, I think modifying post-synaptic variables in "on_post" is already restricted on "+=" etc within brian 2 for obvious reasons?

tnowotny commented 4 years ago

After discussion with @neworderofjamie , he reminded me that our learning kernel in GeNN is parallelised along incoming synapses so that manipulating post-synaptic variables is indeed a bad idea with unpredictable outcomes. However, we inspected the Clopath example and it seems very odd that "on_post" in the synapse equations is used to update a filtered post-synaptic voltage trace in the post-synaptic neuron. As @neworderofjamie pointed out to me, this also means an update of the post-synaptic filtered voltage for every synapse, which seems wrong. Should these traces not be updated in the spike reset code:

reset=('''
      v=V_rest
      x_trace+=x_reset/(taux/ms)
      v_lowpass1 += 10*mV                                   # mimics the depolarisation effect due to a spike
      v_lowpass2 += 10*mV                                   # mimics the depolarisation effect due to a spike
      v_homeo += 0.1*mV 
''')

And then

Post_eq = ('''
            w_plus = A_LTP*x_trace_pre*(v_lowpass2_post/mV - Theta_low/mV)*int(v_lowpass2_post/mV - Theta_low/mV > 0)  # synaptic potentiation
            w_ampa = clip(w_ampa+w_plus, 0, w_max)                                                                     # hard bounds
            ''' )

Or am I having the completely wrong end of the stick @mstimberg ?

tnowotny commented 4 years ago

Last comment for today: the not supported error thrown at https://github.com/brian-team/brian2genn/blob/0c8944b901728ebb55c711cae57f64b36e6c6a27/brian2genn/device.py#L1375 however seems over-zealous as accessing post-synaptic variables read-only is not a problem afaik. Maybe we can relax that, @mstimberg ?

kjohnsen commented 4 years ago

I'm new to Brian and am still reading the paper to try and understand their model, but offhand it seems like updating the low-pass on reset would be functionally equivalent. Though there is a delay between when on_post and when reset are executed, I assume?

mstimberg commented 4 years ago

I agree that putting the update of the low-pass voltage in the reset would make more sense. As @neworderofjamie said, it is a bit weird at the moment because the update will be done for each synapse, i.e. the += 10*mV actually translates into a += 50*mV.

Though there is a delay between when on_post and when reset are executed, I assume?

In "standard Brian", both would be executed in the same time step. According to the schedule, it would check the threshold, propagate the spikes (which includes calling on_post), and execute the reset. In Brian2GeNN this is different, here the synaptic propagation happens in the following time step. But that would only be one more reason to put it into the reset, I think.

@tnowotny :

Last comment for today: the not supported error thrown at [...] however seems over-zealous as accessing post-synaptic variables read-only is not a problem afaik.

I agree, I think we only did this for simplicity....

tnowotny commented 3 years ago

This could be looked at again within GSoC2021.