nengo_gui assumes reference is to an object in the model

[tcstewar]

Consider this code:

import nengo
model = nengo.Network()
with model:
    a = nengo.Ensemble(100, 1)    
    c = nengo.Connection(a, a)    
#model.connections.remove(c)

This works fine. But try uncommenting the last line. nengo_gui doesn't realize that the object that it used to refer to as c is gone from the model, since it still has access to something called c. So it doesn't delete the recurrent connection from the visual display. Note that this is not a problem in this case:

import nengo
model = nengo.Network()
with model:
    a = nengo.Ensemble(100, 1)    
    nengo.Connection(a, a)
model.connections.pop(0)

The part of the code that's causing this is here:

https://github.com/nengo/nengo_gui/blob/master/nengo_gui/components/netgraph.py#L126

Basically, uid is the python text that refers to the item we're currently considering ('c' in this case), and old_item is the object that that text used to refer to (the previous time we evaluated the code), and new_item is the new object. The code then compares the two to see if there's anything we need to do to update the gui.

When I wrote that, I assumed that anything that we can still access still exists in the model, but that's not necessarily the case, especially as I've gotten more and more used to doing helper functions that modify the nengo network.

I'm not sure what to do about this yet. The only correct solution I can think of is to also check that every object is inside the model graph, but that involves doing a scan through the entire model, since it might be nested ridiculously deep. I can cache this scan so that we only have to do it once, but so far I'd been avoiding doing such a complete scan (right now, it only scans into Networks that are not collapsed in the gui).

It's probably not too expensive in terms of time to do that scan, but I was really hoping to avoid it....

[jgosmann]

I think that function needs some refactoring anyways: #767. Also, there is a somewhat related bug that has been fixed in so far that it doesn't crash the GUI anymore, but the underlying code is still not correct: #763 This comment might be most useful in explaining the problem. I remember that I was trying to fix it, but it wasn't easy because the complexity of _reload (and maybe a proper qix would have needed some major refactoring? I don't recall).

Maybe we can solve all those problems at once?

[jgosmann]

The only correct solution I can think of is to also check that every object is inside the model graph, but that involves doing a scan through the entire model, since it might be nested ridiculously deep.

I'm looking a little bit into this and related issues (not sure if I I will fix it though, it will take some effort understanding what is currently done, understand what should be done, and finally refactor and implement those things). But I don't think that a scan through the entire model is a problem. It's happening already anyways: We have to run the model code first which is basically equivalent to one traversal. Of course we shouldn't do that traversal repeatedly, but we can save the network objects in a data structure that gives us constant time for the required operations (__contains__, potentially __getitem__?).

It might also make sense to “invert” the logic of the _reload function. At the moment it is using the old uids to look up stuff in the new model, but it might be better to us the new model to generate new uids and the use those to figure out the required updates. But this might also be a horrible idea; currently I don't understand enough of the _reload function (it is too long and complex).