Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make individual connection output probeable #974

Merged
merged 1 commit into from
Mar 17, 2016
Merged

Conversation

hunse
Copy link
Collaborator

@hunse hunse commented Mar 2, 2016

Fixes #973.

@hunse hunse mentioned this pull request Mar 2, 2016
@tbekolay tbekolay added the bug label Mar 2, 2016
@tbekolay tbekolay added this to the 2.1.0 release milestone Mar 2, 2016
@tbekolay
Copy link
Member

tbekolay commented Mar 2, 2016

This LGTM and as a bugfix makes sense for 2.1. Can you add a changelog entry?

@tbekolay
Copy link
Member

tbekolay commented Mar 2, 2016

A test would also be good; I can add that if you want.

@hunse
Copy link
Collaborator Author

hunse commented Mar 2, 2016

Sure, if you have a good idea for a test go for it. I'll add a changelog entry.

@tbekolay
Copy link
Member

Rebased to master and added a test. @drasmuss can you review?

@drasmuss
Copy link
Member

I think it might make sense to include the slice in the probed output. I.e., if I do

conn = nengo.Connection(a, b[0])
probe = nengo.Probe(conn)

I think I would expect the probed values to be 1D. Something like this instead:

      # Store the weighted-filtered output in case we want to probe it
      model.sig[conn]['weighted'] = signal[post_slice]

@hunse
Copy link
Collaborator Author

hunse commented Mar 16, 2016

The slice is on where the function is going, though, not on the function itself. So I'm fine with leaving off the output slice. In your example, the probed values HAVE to be 1D, because your connection code is saying "compute this function and put it into the first dimension of 'b'", and since the first dimension of 'b' is 1D then the probed thing has to be 1D. So the output slice isn't being applied to the output of the connection, it's being applied to where the connection's output is going. Does that make sense?

@drasmuss
Copy link
Member

Ah yes of course you're right, I just glanced at the

model.add_op(SlicedCopy(
          signal, model.sig[conn]['out'], b_slice=post_slice,
         inc=True, tag="%s.gain" % conn))

right beneath and was thinking "we should have that in", but of course the slicing is being done by the weights already. All looks good then!

@tbekolay tbekolay merged commit 3ad892e into master Mar 17, 2016
@tbekolay tbekolay deleted the probe-conn-output branch March 17, 2016 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants