brian-team / brian2

Brian is a free, open source simulator for spiking neural networks.
http://briansimulator.org
Other
936 stars 220 forks source link

Allow explicit array access in string syntax #388

Open thesamovar opened 9 years ago

thesamovar commented 9 years ago

A use case was suggested to me for explicit array access within strings. This is when you want to combine mathematically defined and recorded data into a model. For example, you might have recorded local connectivity structures, but join them up globally via a mathematical model. I think it would be sufficient to allow string syntax like the following: arr[i%N]*f(i,j). In other words, we'd allow flat 1D arrays and [] syntax. Thoughts?

mstimberg commented 9 years ago

To be clear, we are talking about using an externally defined array, right? So something like:

arr = np.arange(10)
G = NeuronGroup(20, 'v:1')
G.v = 'arr[i % N]*mV'

We can already do something like this in a complicated way with a dummy NeuronGroup and an index linked variable, but this is certainly not very convenient. It makes me think though whether we should use the same syntax in both cases, i.e. allow replacing the following example from the docs:

# two inputs with different phases
inp = NeuronGroup(2, '''phase : 1
                        dx/dt = 1*mV/ms*sin(2*pi*100*Hz*t-phase) : volt''')
inp.phase = [0, pi/2]

neurons = NeuronGroup(100, '''inp : volt (linked)
                              dv/dt = (-v + inp) / tau : volt''')
# Half of the cells get the first input, other half gets the second
neurons.inp = linked_var(inp, 'x', index=repeat([0, 1], 50))

by

# two inputs with different phases
inp = NeuronGroup(2, '''phase : 1
                        dx/dt = 1*mV/ms*sin(2*pi*100*Hz*t-phase) : volt''')
inp.phase = [0, pi/2]

neurons = NeuronGroup(100, '''inp : volt (linked)
                              dv/dt = (-v + inp[i%2]) / tau : volt''')

It's nice to have the indexing as part of the equations, on the other hand we can't really do error checking any more if we allow arbitrary expressions for the indexing, and this could lead to segmentation faults in weave/cython/standalone.

Alternatives I could think of:

thesamovar commented 9 years ago

Ah yes, there are clearly some problems. We could think about having a 'debug' mode with bounds checking? Generally speaking, I'd be more in favour of adding string expressions to linked_var rather than restricting to array indexing. Wasn't there a reason we didn't do this though? I don't remember.

I also realised that the current linked variable technique could potentially cause problems with assumptions made by state updaters? For example, using it you could couple v[0] and v[1] in the equations, and then the order in which they are updated will change their values. I wonder if there's anything we can do about this?

mstimberg commented 9 years ago

Ah yes, there are clearly some problems. We could think about having a 'debug' mode with bounds checking

That's an option. It would break a bit our current approach where we do all general syntax/consistency checking before a run, i.e. you should never get a runtime or compiler error (except for the case where you provide the source code for a function).

Generally speaking, I'd be more in favour of adding string expressions to linked_var rather than restricting to array indexing. Wasn't there a reason we didn't do this though? I don't remember.

It would be neat, the problem I'm seeing is that it is not compatible with our current indexing approach, where in the generated code block we first generate the index variables/arrays, then index the state variables with the arrays and finally use the user-provided code without any indices. If the index can consist of arbitrary string expressions, we might not have all the relevant variables yet at this point.

I also realised that the current linked variable technique could potentially cause problems with assumptions made by state updaters? For example, using it you could couple v[0] and v[1] in the equations, and then the order in which they are updated will change their values. I wonder if there's anything we can do about this?

Good point, indeed I think that if you link a state variable within a group, you might already get different behaviour with numpy and weave, in numpy you'll strictly use the values from the previous timestep and in weave you use whatever the current value is in the loop. Maybe we should forbid linking to a variable in the same group? On the other hand, this is used in brian2hears for the temporal shifting (which currently assumes that you go through the state update step in the correct order).

thesamovar commented 9 years ago

we do all general syntax/consistency checking before a run, i.e. you should never get a runtime or compiler error

I think we should try to keep this as much as possible, but if we decide that the functionality is worth having it's OK to break this. (As we do for functions, for example.)

If the index can consist of arbitrary string expressions, we might not have all the relevant variables yet at this point.

Right, so it's a big change so we need to be sure we actually want to do it. I'd say it's probably not a top priority at the moment, but maybe it's one of those things that it's better to put in earlier rather than later? It does allow for a lot of flexibility which I'm generally in favour of.

I think that if you link a state variable within a group, you might already get different behaviour with numpy and weave

Yes, I think so too. Perhaps we should have a warning rather than an error? And allow a keyword argument to switch that warning off (so that brian2hears can switch it off without switching it off globally).

Is there any way to do correctness/consistency checking on these sorts of expressions? My guess is that it would be a hard problem and we'd be better off just having a general warning and allow the user to shoot themselves in the foot if they insist.