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

Summed post-synaptic variables accessed from synapse code #57

Closed
tnowotny opened this issue Apr 25, 2018 · 19 comments
Closed

Summed post-synaptic variables accessed from synapse code #57

tnowotny opened this issue Apr 25, 2018 · 19 comments

Comments

@tnowotny
Copy link
Contributor

Allow this currently unsupported functionality of Brian 2, including targeting more than one post-synaptic variable in a summed fashion.

@tnowotny
Copy link
Contributor Author

@schlowm0 - thanks for reminding about these issues. I have started to look into this but I am not quite sure any more what exactly it is you were after.
In particular, one might want to do
(1)

V+= s  (summed)
w*=s (summed)

where s is a synaptic variable and V and w are variables of the post-synaptic neuron.
(2)

V+=s1 (summed)
w*=s2 (summed)

with two synaptic variables s1 and s2 being summed individually.

Case (1) we can support now fairly trivially. However, (2) would need some thinking and changes in GeNN itself I believe. @neworderofjamie, feel free to comment!

@neworderofjamie
Copy link
Collaborator

Totally agree - 1 should be simple and 2 requires more work (as detailed in genn-team/genn#160)

@schlowm0
Copy link

schlowm0 commented Feb 14, 2019

Thanks for looking into the issue again.
So the summed issue is a known issue of Brian2. In our toolbox teili we overcome this issue by registering all neuronal inputs at the creation of the network meaning that the synapses follow the following eq:

Ie{input_number}_post = Ie_syn : amp (summed)

where we assign for each synapttic projection a input_number and the neuron than simply sums all registered variables into its own variable:

Iin = Ie0+Ii0 : amp # input currents

So I believe if GeNN would offer an option that the underlying code takes care of the summation and thus do not throw an error when trying to sum at the synapses, it should be enough. At least if all to be summed variables are registered correctly.

Note: We simulate the current-mode circuits of the analog neuromorphic chips of Giacomo, that's why we have currents as variables.

@schlowm0
Copy link

So maybe to explain again why we opened this issue.
I wanted to use brian2GeNN for some large-scale simulation. In our toolbox we decided to sum the synaptic currents in the synapse and then sum all summed synaptic inputs in the neuron.
When one does this brian2genn throws an error that this style is not possible due to a brian2 limitation. However we overcome the brian2 limitation in our toolbox and thus we take care of the summing issue .

I think that GeNN internally just throws an exception/error when the code performs summing operation as a safety switch to prevent execution of non-brian2 supported functionality. But in our toolbox we support this functionality and thus GeNN should not throw the exception.

@neworderofjamie
Copy link
Collaborator

neworderofjamie commented Feb 14, 2019

Ok, I think between @tnowotny and myself we understand the problem! After #82, you should be able to implement this by making the following changes to the model template (I'm afraid my Brian2GeNN knowledge doesn't go much further up the stack than this):

  1. Somewhere around https://github.com/brian-team/brian2genn/blob/master/brian2genn/templates/model.cpp#L40, add something resembling SET_ADDITIONAL_INPUT_VARS({{"Ie0", {"scalar", 0.0}}, {"Ii0", {"scalar", 0.0}}});
  2. At https://github.com/brian-team/brian2genn/blob/master/brian2genn/templates/model.cpp#L94, change $(Isyn) to $(Ie0) and $(Ii0) depending on the synaptic input in question.

Then in the neuron code scalar Iin = Ie0+Ii0 would work as expected (I can't remember why they aren't $(Ie0) and $(Ii0))

@schlowm0
Copy link

Just to clarify, would I need to do the change locally and do it every time a want to simulate a specific model? Because depending on the inputs we have more or less Ie's and Ii's...

@tnowotny
Copy link
Contributor Author

No, we need to sort this out internally in brian2genn. In addition to what Jamie wrote, we also need to change the code that detects which post-synaptic variables are targeted by which synapse group and then select the correct code path (still warning and aborting if two different synaptic variables are summed in the same synapse population - this remains a bottleneck). I will need Marcel's help but will have a look at this when I get a chance. In Capocaccia at the very latest.

@mstimberg
Copy link
Member

Hi, I'm happy to help from Brian2GeNN's side, but I'm not 100% sure I understand what you want to do. Do you want to have Brian2GeNN automatically take care of brian-team/brian2#766, i.e. make it possible for the user to target the same post-synaptic variable several times, and under the hood it gets replaced by multiple variables with automatic names that are summed together in a separate expression? Or does Brian2GeNN not support this workaround at the moment and this issue is about fixing this?

@schlowm0
Copy link

schlowm0 commented Feb 15, 2019

In our toolbox we implement a solution to register multiple inputs to be integrated on the same post-synaptic variable. Brian2GeNN reports that it does not support this feature. However, it does not need to support it, only to accept that we already took care of this.

I think it sees in our code that we have the summed flag at the synapses and that's why it throws an error.

@mstimberg
Copy link
Member

Ok, thanks for the clarification!

@schlowm0
Copy link

So we need a mechanism/flag to tell Brian2GenNN that it should accept our code... I think, but please correct me @tnowotny

@tnowotny
Copy link
Contributor Author

Hi @mstimberg , my reading is that in order to support @schlowm0 's "hack", we need to allow separate synapse objects to target different post-synaptic neuron variables. This is something we can do in GeNN using the new GeNN syntax elements @neworderofjamie has pointed out. All that is missing is to detect within brian2genn that indeed only one synaptic variable per synapse object is summed and make sure that synapse objects either all create their own post-synaptic target variable (or all but one, who could use the standard "Isyn" variable).
I hope it makes more sense now. I am happy to start poking around but usually @mstimberg , you have the better overview of where to attack first.

@tnowotny
Copy link
Contributor Author

tnowotny commented Feb 21, 2019

Ok, @mstimberg , I was a little on the wrong track here. The main problem appears to be that we need to simply support summed variables.
I have looked into it in some detail and I think it can be done but I am missing a couple of steps.
My summary so far:

  1. We need to switch off that we are throwing an NotImplementedError if an object in a network has a 'summed_variable' template, here:
    for obj in net.objects:
    if hasattr(obj, 'template'):
    if obj.template in ['summed_variable']:
    raise NotImplementedError(
    'The function of %s is not yet supported in GeNN.' % obj.template)
  2. For each of such object we need to add a line of the following type to the matching synapse_model.main_code_lines['dynamics']:
$(addToInSyn,$({{synaptic_var}}));\n

and to the synapse_model.postSyntoCurrent:

0; $({{obj.target_var.name}})= $(inSyn);

we should create synapse_model.decayCode:

$(inSyn)= 0.0; 

and add here:

SET_DECAY_CODE("{% for line in synapse_model.decayCode %}{{line}}{% endfor %};");

I think that should about do it.
I would implement it myself but unfortunately I am stuck at the front: When are the network objects with the summed_variable template processed and how do we channel them most effectively to populate the synapse_model objects that we normally populate in process_synapses (i.e. here:

def process_synapses(self, synapse_groups):

? It's probably trivial but I am still a bit lost in the Brian 2 code generation pathways.
@neworderofjamie - any comments welcome from your side as well.

@tnowotny
Copy link
Contributor Author

Oh - and one more thing. We should also at some point in the code generation pipeline check that for a given Synapses object, in their totality all pre code objects and all summed_variable objects altogether target one and only one post-synaptic variable. They could do that in several places (all would be added together) but only for one post-synaptic variable as it all goes through inSyn.

@mstimberg
Copy link
Member

Hi, sorry, I didn't yet have much time to look into this.

When are the network objects with the summed_variable template processed and how do we channel them most effectively to populate the synapse_model objects that we normally populate in process_synapses

I think we'll have to use the same approach as when we handle thresholds, resets, etc., i.e. "code objects" that are independently treated in Brian, but part of the neuron definition in GeNN. In the code_object function, we include the summed_variable template in the list of templates for which we create a DelayedCodeObject (which basically means: "do nothing"):

if (template_name in ['stateupdate', 'threshold', 'reset'] and
isinstance(owner, NeuronGroup)):
# Delay the code generation process, we want to merge them into one
# code object later
codeobj = DelayedCodeObject(owner=owner,
name=name,
abstract_code=abstract_code,
variables=variables,
variable_indices=variable_indices,
override_conditional_write=override_conditional_write)
self.simple_code_objects[name] = codeobj

Later in process_synapses, we can go through the list of all objects and take out the SummedVariabledUpdater objects one at a time, similar to how we search for the run_regularly objects here:

run_regularly_objects = [o for o in objects
if o.startswith(obj.name + '_run_regularly')]

All other checks can be done in process_synapses as well, I think.

@tnowotny
Copy link
Contributor Author

Thanks ... that makes a lot of sense. Would the owner of a summed_variable template be an instance of a NeuronGroup? Otherwise we might have to duplicate the code for SynapseGroup owners ...
I will get back to this next week.

@mstimberg
Copy link
Member

For code like this:

neurons = NeuronGroup(1, """dv/dt = (g-v)/(10*ms) : 1
                            g : 1""", method='exact')
S = Synapses(input, neurons,'''
                dg_syn/dt = ... : 1 (clock-driven)
                g_post = g_syn : 1 (summed)''')

there will be a SummedVariableUpdater object (named 'synapses_summed_variable_g_post'). It has an owner attribute referring to the synapses S, a target attribute referring to the NeuronGroup neurons, a target_var attribute storing a reference to the Variable object representing g in the NeuronGroup (in your case, you'd be only interested in target_var.name which is 'g' in this case), and finally abstract_code storing '_synaptic_var = g_syn'. The synaptic_var is a standard name, only the right-hand side will change depending on the definition of the summed variable by the user. This should be all you need, I guess?

@tnowotny
Copy link
Contributor Author

I think that's all - thanks a lot ... I will try to find some time to do the changes very soon. I think it should be quite easy after you provided this information (famous last words ...).

@schlowm0
Copy link

schlowm0 commented Apr 30, 2019

So I tried your minimal working example @tnowotny and it worked :)
The example Thomas provided can be found here.

I can confirm that it also works with the models provided within teili
The example I test looks like this:

import os
import numpy as np

from brian2 import ms, nA, SpikeGeneratorGroup,\
    SpikeMonitor, StateMonitor, prefs, set_device,\
    asarray

import brian2genn

from teili import TeiliNetwork, Neurons, Connections
from teili import DPI, DPISyn
from teili.models.parameters.dpi_neuron_param import parameters as DPIparam
from teili import SynapseEquationBuilder

path = os.path.expanduser("~")
model_path = os.path.join(path, "teiliApps", "equations", "")

synapse_obj = SynapseEquationBuilder.import_eq(
    model_path + 'DPISyn.py')

set_device('genn', use_GPU= False, directory='teili2genn_test', debug=True)

tsInp = asarray([1, 3, 4, 5, 6, 7, 8, 9]) * ms
indInp = asarray([0, 0, 0, 0, 0, 0, 0, 0])
gInpGroup = SpikeGeneratorGroup(1, indices=indInp,
                                times=tsInp, name='gtestInp')

Net = TeiliNetwork()

testNeurons = Neurons(2, equation_builder=DPI(num_inputs=2), name="testNeuron")
# Example of how to set parameters, saved as a dictionary
testNeurons.set_params(DPIparam)
testNeurons.refP = 3 * ms

testNeurons2 = Neurons(2, equation_builder=DPI(num_inputs=2), name="testNeuron2")
testNeurons2.refP = 3 * ms

InpSyn = Connections(gInpGroup, testNeurons,
                     equation_builder=synapse_obj,
                     name="testSyn", verbose=False)
InpSyn.connect(True)

InpSyn.weight = 10
Syn = Connections(testNeurons, testNeurons2,
                  equation_builder=synapse_obj,
                  name="testSyn2")
Syn.connect(True)

# you can change all the parameters like this after creation of the neurongroup:
Syn.weight = 100

# Example of how to set single parameters, rather than using an entire dictionary
testNeurons.Iconst = 10 * nA

Net.add(gInpGroup, testNeurons, testNeurons2, InpSyn, Syn)
duration = 500
Net.run(duration * ms)

However, I encountered the following error when using Spike and/or Statemonitors which looks like this:

NotImplementedError: Recording an expression that depends on the time t or the timestep dt is currently not supported in Brian2GeNN

as this is not strictly related to this issue I opened a new issue #86
I propose to close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants