Returned value needs to be retrieved for object to work

[jlubo]

When defining a PoissonInput object, the value returned by the constructor needs to be retrieved for the object to work. While I think that this is not a big issue, it can be confusing because one wouldn't expect the retrieval of the object to make any difference. The same issue might exist for other object types like NeuronGroup, SpikeGeneratorGroup, etc, but for those the practical relevance should be small because the returned object is usually needed by the subsequent code.

Here is an example of what works and what doesn't (altered code blocks for this example):

# Case 1: retrieval of returned object -- WORKS
external_poisson_input = PoissonInput(
    target=neurons, target_var="v", N=C_ext, rate=nu_ext, weight=J
)

# Case 2: no retrieval of returned object -- DOES NOT WORK
PoissonInput(
    target=neurons, target_var="v", N=C_ext, rate=nu_ext, weight=J
)

# Case 3: retrieval of returned object with placeholder -- WORKS
_ = PoissonInput(
    target=neurons, target_var="v", N=C_ext, rate=nu_ext, weight=J
)

I guess a solution to the issue may be to simply throw a warning if the returned value is not retrieved (in case 2).

[mstimberg]

Hi Jannik,
I'd say that this is working as intended, actually. A PoissonInput is an element of a network (as a NeuronGroup, etc.) and therefore needs to be added to the network. If you call run, it uses the visible variables (i.e. the variables in the current scope) to decide which objects to include in the network. The other option is to explicitly construct your Network object yourself, but in this case you'd also need a variable storing the PoissonInput object, or at least do something like this:

net = Network()
# ...
net.add(PoissonInput(...))

See https://brian2.readthedocs.io/en/stable/user/running.html#networks

During the early days of Brian 2 we decided for this mechanism and against a system that would do what you expected here, i.e. something that tracks all the objects that have been created and adds them to the network. One of the reasons is that it makes it more straightforward to have independent simulations, such as in:

def sim1():
   group1 = NeuronGroup(...)
   run(...)
def sim2():
   group2 = NeuronGroup(...)
   run(...)

# Run simulations
sim1()
sim2()

With the current system, sim2 will construct a new network with group2 in it, but a system based on object creation would first have to figure out that group1 should be discarded.

Long story short, I think there are pro and cons for different approaches, but I don't think we can revert this basic decision anytime soon (i.e., before creating Brian 3 :) ). All that said, PoissonInput could be considered a corner case similarly to a run_regularly operation – calling run_regularly returns an object that represents the operation, but you don't have to add it to the network. There might be room for a parallel syntax for PoissonInput, which would get rid of the requirement to store a reference to the object:

neurons.add_poisson_input(
    target_var="v", N=C_ext, rate=nu_ext, weight=J
)

[jlubo]

Thanks, Marcel, for the elaboration! I agree that the concept is fine in general. It may just have to be pointed out to users that the object reference needs to be retrieved.

I guess implementing a special case for PoissonInput as you suggested would be a good solution. Otherwise, what do you think about simply counting PoissonInput instances and throwing a warning? Maybe that would be easier, but I'm not sure.

[jlubo]

By the way, a related issue would be that Brian doesn't complain if the same variable name is used more than once to store objects like NeuronGroup:

# Case 1: same variable name is used more than once -- DOES NOT WORK
ng = NeuronGroup(...)
...
ng = NeuronGroup(...)

# Case 2: unique variable names are used for all objects -- WORKS
ng = NeuronGroup(...)
...
ng2 = NeuronGroup(...)

I think there should be a warning for this as well, which could maybe also be implemented by counting the objects.

[mstimberg]

Otherwise, what do you think about simply counting PoissonInput instances and throwing a warning? Maybe that would be easier, but I'm not sure.

The problem with this is that we want to keep track of instances that are not stored in variables, then we need to keep references to these objects ourselves. Brian 1 actually had a mechanism like that, but it means that if you do e.g. a loop where you re-create objects, then the memory for the unused objects never gets freed.

By the way, a related issue would be that Brian doesn't complain if the same variable name is used more than once to store objects like NeuronGroup:

I think this has the same issues as above – reusing the same variable name is quite common (e.g. when you do a loop, or when you use a function to create objects), and keeping references to all created objects would create memory leaks.

Thinking about this a bit more, there might be a way to do something along these lines by hooking into the remove method in InstanceTrackerSet. This is already tracking all objects that get created, but using weak references to avoid the issues discussed above. We could make Network notify this object that a certain object had been included in a network, and inversely, raise a warning if an object gets removed without ever having been included in a Network. I'm a bit hesitant about whether this is worth it, i.e. whether it would be more often helpful than confusing. But maybe worth trying out?

[jlubo]

I see the issues... As far as I can tell, the solution that you propose sounds very solid to me. And I guess it will be helpful for many people - probably many are at some point stuck with their Brian code because they don't see what's wrong.

If I find some time, I could also try to implement a solution myself and create a PR.

[mstimberg]

I have to say I wasn't 100% convinced that this happens very often, but then this morning a post on Discourse described a situation where this warning would have been triggered https://brian.discourse.group/t/synapses-not-functioning/1118 🙃

[jlubo]

And that was not arranged 😀