-
Notifications
You must be signed in to change notification settings - Fork 22
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
Update to support core SimPES changes #181
Conversation
9e1e3eb
to
f223781
Compare
@@ -1230,16 +1230,18 @@ def special_set(s, op): | |||
# reshape view | |||
self.signals[sig] = self.signals[sig.base].reshape(sig.shape) | |||
else: | |||
# slice view | |||
|
|||
if sig.shape[1:] != sig.base.shape[1:]: | |||
# TODO: support this? | |||
raise NotImplementedError("Slicing on axes > 0 is not supported") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it would be too hard to do this, and with the code below getting more complicated, it might actually be clearer just to implement it. That is, unless there's some underlying reason TensorFlow can't support it?
Doesn't have to happen in this PR, but something to keep in mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there were any other situations where this functionality was used then I'd probably look into implementing this at that point, but as long as it's only used in SimPES it seems easier to just special-case it there.
aa6cf5d
to
7c9e1bb
Compare
Raise an error if advanced indexing is used, and fix the same bug found in nengo core (when applying PES learning on a neuron-to-neuron connection with a post slice).