{"action":"opened","issue":{"url":"https://api.github.com/repos/nengo/nengo/issues/1234","repository_url":"https://api.github.com/repos/nengo/nengo","labels_url":"https://api.github.com/repos/nengo/nengo/issues/1234/labels{/name}","comments_url":"https://api.github.com/repos/nengo/nengo/issues/1234/comments","events_url":"https://api.github.com/repos/nengo/nengo/issues/1234/events","html_url":"https://github.com/nengo/nengo/issues/1234","id":195118033,"number":1234,"title":"Inconsistent behaviour between connection probes and signal probes","user":{"login":"drasmuss","id":1952220,"avatar_url":"https://avatars.githubusercontent.com/u/1952220?v=3","gravatar_id":"","url":"https://api.github.com/users/drasmuss","html_url":"https://github.com/drasmuss","followers_url":"https://api.github.com/users/drasmuss/followers","following_url":"https://api.github.com/users/drasmuss/following{/other_user}","gists_url":"https://api.github.com/users/drasmuss/gists{/gist_id}","starred_url":"https://api.github.com/users/drasmuss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/drasmuss/subscriptions","organizations_url":"https://api.github.com/users/drasmuss/orgs","repos_url":"https://api.github.com/users/drasmuss/repos","events_url":"https://api.github.com/users/drasmuss/events{/privacy}","received_events_url":"https://api.github.com/users/drasmuss/received_events","type":"User","site_admin":false},"labels":[{"id":36691245,"url":"https://api.github.com/repos/nengo/nengo/labels/bug","name":"bug","color":"000000","default":true}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2016-12-12T23:35:52Z","updated_at":"2016-12-12T23:35:52Z","closed_at":null,"body":"Here's a minimal example illustrating the issue:\r\n``` python\r\nimport nengo\r\n\r\nwith nengo.Network(seed=0) as net:\r\n    ens = nengo.Ensemble(5, 1, neuron_type=nengo.Sigmoid())\r\n\r\n    input_p = nengo.Probe(ens.neurons, attr=\"input\", synapse=0)\r\n    output_p = nengo.Probe(ens.neurons, attr=\"output\", synapse=0)\r\n\r\nwith nengo.Simulator(net) as sim:\r\n    sim.run(0.002)\r\n```\r\n\r\nThe difference between `synapse=0` and `synapse=None` is that synapses introduce a one-step time delay (under the hood this is because they are `update` operations, so they run \"after\" the simulation step).  So we would expect both `input_p` and `output_p` to be delayed by one timestep above.  But if we run it, we see\r\n```\r\n[[  0.00000000e+00   0.00000000e+00   0.00000000e+00   0.00000000e+00\r\n    0.00000000e+00]\r\n [  2.70755224e+01   1.35455073e-02   7.22264321e+00   1.56976494e+02\r\n    3.34045344e+01]]\r\n```\r\nfor `output_p` (delayed as we would expect), and\r\n```\r\n[[ -2.86030562 -10.51628136  -4.22283648  -0.7817029   -2.63677099]\r\n [ -2.86030562 -10.51628136  -4.22283648  -0.7817029   -2.63677099]]\r\n```\r\nfor `input_p` (no delay).  We can confirm this by setting `synapse=None` on both probes, in which case we see `input_p` is unchanged (no delay, but that is what we expect this time), and `output_p` changes to \r\n```\r\n[[  2.70755224e+01   1.35455073e-02   7.22264321e+00   1.56976494e+02\r\n    3.34045344e+01]\r\n [  2.70755224e+01   1.35455073e-02   7.22264321e+00   1.56976494e+02\r\n    3.34045344e+01]]\r\n```\r\n(the delay disappears, again as we would expect).\r\n\r\nSo basically the issue is that the `input_p` should show a one-step time delay when `synapse=0`, but it doesn't.  \r\n\r\nThe reason is that `output_p` is built as a \"connection probe\", meaning that we copy the value of `neurons.output` to a new signal.  This copy happens before the filter `update` is applied (at the end of the timestep). \r\n\r\n`input_p`, on the other hand, is a \"signal probe\" (it just contains a direct reference to the target signal, which in this case is `neurons.input`).  The probe signal and `neurons.input` are aliased, so that when the filter `update` runs at the end of the timestep, it also updates the probed value.  So for `output_p` we're seeing the value of the target signal before the filter update, and for `input_p` we're seeing the value of the target signal after the filter update.\r\n\r\nIt's a fairly easy fix, we just change `builder/probe.py:signal_probe` to\r\n``` python\r\ndef signal_probe(model, key, probe):\r\n    # Signal probes directly probe a target signal\r\n\r\n    try:\r\n        sig = model.sig[probe.obj][key]\r\n    except IndexError:\r\n        raise BuildError(\r\n            \"Attribute %r is not probeable on %s.\" % (key, probe.obj))\r\n\r\n    if probe.slice is not None:\r\n        sig = sig[probe.slice]\r\n\r\n    if probe.synapse is None:\r\n        model.sig[probe]['in'] = sig\r\n    else:\r\n        ### new part ###\r\n        model.sig[probe]['filtered'] = model.build(probe.synapse, sig)\r\n        model.sig[probe]['in'] = Signal(np.zeros(sig.shape), name=str(probe))\r\n        model.add_op(SlicedCopy(model.sig[probe]['filtered'],\r\n                                model.sig[probe]['in']))\r\n```\r\n\r\nHowever, this causes a unit test to fail, `test_learning_rules.py:test_dt_dependence`.  That test was somewhat relying on the above behaviour, because signal probes are more dt independent when you don't have these one-step time delays.  But I don't think the intention of the test was to check the dt independence of signal probes, it was to check the dt independence of learning rules.  So if we just set `synapse=None` on that signal probe (removing that part of the test), it passes fine.\r\n\r\nSo, that would be my proposed solution.  However, it is kind of unusual to make a change that breaks a formerly passing unit test, so I thought I'd give people a chance to weigh in.  You could perhaps make an argument that the current implementation is working as intended, and we shouldn't expect signal and connection probes to work the same way.  But since that distinction is totally hidden from the frontend user, I think this change is for the better."}}
{"actor":{"display_login":"drasmuss"}}