{"action":"created","issue":{"url":"https://api.github.com/repos/nengo/nengo/issues/1152","repository_url":"https://api.github.com/repos/nengo/nengo","labels_url":"https://api.github.com/repos/nengo/nengo/issues/1152/labels{/name}","comments_url":"https://api.github.com/repos/nengo/nengo/issues/1152/comments","events_url":"https://api.github.com/repos/nengo/nengo/issues/1152/events","html_url":"https://github.com/nengo/nengo/issues/1152","id":173800975,"number":1152,"title":"Neurons object (and related) should not use a weak reference to ensemble","user":{"login":"jgosmann","id":850157,"avatar_url":"https://avatars.githubusercontent.com/u/850157?v=3","gravatar_id":"","url":"https://api.github.com/users/jgosmann","html_url":"https://github.com/jgosmann","followers_url":"https://api.github.com/users/jgosmann/followers","following_url":"https://api.github.com/users/jgosmann/following{/other_user}","gists_url":"https://api.github.com/users/jgosmann/gists{/gist_id}","starred_url":"https://api.github.com/users/jgosmann/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jgosmann/subscriptions","organizations_url":"https://api.github.com/users/jgosmann/orgs","repos_url":"https://api.github.com/users/jgosmann/repos","events_url":"https://api.github.com/users/jgosmann/events{/privacy}","received_events_url":"https://api.github.com/users/jgosmann/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/nengo/nengo/labels/bug","name":"bug","color":"000000"},{"url":"https://api.github.com/repos/nengo/nengo/labels/discussion","name":"discussion","color":"207de5"}],"state":"open","locked":false,"assignee":{"login":"jgosmann","id":850157,"avatar_url":"https://avatars.githubusercontent.com/u/850157?v=3","gravatar_id":"","url":"https://api.github.com/users/jgosmann","html_url":"https://github.com/jgosmann","followers_url":"https://api.github.com/users/jgosmann/followers","following_url":"https://api.github.com/users/jgosmann/following{/other_user}","gists_url":"https://api.github.com/users/jgosmann/gists{/gist_id}","starred_url":"https://api.github.com/users/jgosmann/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jgosmann/subscriptions","organizations_url":"https://api.github.com/users/jgosmann/orgs","repos_url":"https://api.github.com/users/jgosmann/repos","events_url":"https://api.github.com/users/jgosmann/events{/privacy}","received_events_url":"https://api.github.com/users/jgosmann/received_events","type":"User","site_admin":false},"assignees":[{"login":"jgosmann","id":850157,"avatar_url":"https://avatars.githubusercontent.com/u/850157?v=3","gravatar_id":"","url":"https://api.github.com/users/jgosmann","html_url":"https://github.com/jgosmann","followers_url":"https://api.github.com/users/jgosmann/followers","following_url":"https://api.github.com/users/jgosmann/following{/other_user}","gists_url":"https://api.github.com/users/jgosmann/gists{/gist_id}","starred_url":"https://api.github.com/users/jgosmann/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jgosmann/subscriptions","organizations_url":"https://api.github.com/users/jgosmann/orgs","repos_url":"https://api.github.com/users/jgosmann/repos","events_url":"https://api.github.com/users/jgosmann/events{/privacy}","received_events_url":"https://api.github.com/users/jgosmann/received_events","type":"User","site_admin":false}],"milestone":null,"comments":3,"created_at":"2016-08-29T15:25:47Z","updated_at":"2016-08-29T17:16:54Z","closed_at":null,"body":"Try the following code:\r\n```python\r\nneurons = nengo.Ensemble(10, 1, add_to_container=False).neurons\r\nprint(neurons.ensemble)\r\n```\r\n\r\nIt will print `None` because the only remaining reference to the ensemble is a weak reference. This means the `neurons` object cannot be used in connections anymore. This might seem like an irrelevant case because the ensemble is never added to a network, but this is just the simplest piece of code reproducing the problem. In the tests in #984 (added by @hunse) the same problem occurs because the network reference is not kept around (that's why I found this problem in the first place).\r\n\r\nIt would be possible to fix the tests to keep around the network reference, but to me it feels like this should not be necessary. I could also see, this crop up and various other place if people make use of copies.\r\n\r\nTo me it seems better to change the weak reference into a strong reference. This would fix the problem without the risk of running into it again and it probably would simplify the copy code (because it won't have to deal with weak references only with shallow vs deep copies). There is a slight disadvantage, though: The `Ensemble` and `Neurons` objects will have a reference cycle that prevents memory deallocation when all other references to those objects are gone. But this is not a huge problem as the Python garbage collector will dispose of the objects; it's just a later point in time.\r\n\r\nDoes anyone have objections or did I miss any reasons for using weak references?\r\n\r\nThe same problem should also be fixed for learning rules and potentially a few other places."},"comment":{"url":"https://api.github.com/repos/nengo/nengo/issues/comments/243189653","html_url":"https://github.com/nengo/nengo/issues/1152#issuecomment-243189653","issue_url":"https://api.github.com/repos/nengo/nengo/issues/1152","id":243189653,"user":{"login":"tbekolay","id":203709,"avatar_url":"https://avatars.githubusercontent.com/u/203709?v=3","gravatar_id":"","url":"https://api.github.com/users/tbekolay","html_url":"https://github.com/tbekolay","followers_url":"https://api.github.com/users/tbekolay/followers","following_url":"https://api.github.com/users/tbekolay/following{/other_user}","gists_url":"https://api.github.com/users/tbekolay/gists{/gist_id}","starred_url":"https://api.github.com/users/tbekolay/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tbekolay/subscriptions","organizations_url":"https://api.github.com/users/tbekolay/orgs","repos_url":"https://api.github.com/users/tbekolay/repos","events_url":"https://api.github.com/users/tbekolay/events{/privacy}","received_events_url":"https://api.github.com/users/tbekolay/received_events","type":"User","site_admin":false},"created_at":"2016-08-29T17:16:54Z","updated_at":"2016-08-29T17:16:54Z","body":"I don't see how making this a non-weakref solves anything... to me it seems like it will cause more bugs, because if you were to deepcopy the `Neurons`, which deepcopies the `Ensemble`, but then elsewhere you also deepcopy the `Ensemble`, then your model is in a super weird state that will still build and run, but be undeniably wrong. The fact of the matter is that copying Nengo models is hard, and that's why we didn't do it until now.\r\n\r\n> To me this seems to be premature optimization.\r\n\r\nI can see how you might think that, but we use `weakref`s because several years ago not using `weakref`s resulted in a simple model build taking several hours. It's not necessarily this particular weakref that solved it, but whenever it _should_ be a weakref we do it now in order to be more careful and reduce the possibility of bugs.\r\n\r\nFor some more context, we used to rely very heavily on deepcopying networks. Prior to 74ea02053fa557b4ed8300e9a3b1121422fd238f, the builder did a deepcopy of the model at the start of the build process so that the builder could modify any of the Nengo objects without worrying that the user's initial model would be changed. Obviously that seems kind of ludicrous now, but Nengo 1.4 was very different, and the model also kept the simulation state in it, and a lot of why we wrote Nengo 2.0 was to decouple the model creation from the simulator. Anyway, so I had the model be deepcopied before simulating so that if anyone in a Nengo 1.4 mindset modified objects during the simulation it wouldn't affect the model they had created in their script, meaning they could create a new simulator on that model and the two simulators could both run at the same time (which is not something you could do in Nengo 1.4).\r\n\r\nDeepcopying worked fine until we were adding more features, building larger models etc, and at one point some combination of features and model size meant that the build was taking essentially forever, and in debugging it I found that there was some reference cycle that was making the deepcopy recuse infinitely, at which point I learned about `weakref`s and put them in where it made sense.\r\n\r\nRemoving `weakref`s where we decided a `weakref` made sense and have been using problem-free up until now seems like a step backward. I am highly skeptical that it makes the copy code easier while also not causing subtle bugs..."}}
{"actor":{"display_login":"tbekolay"}}