-
Notifications
You must be signed in to change notification settings - Fork 174
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
Added function to evaluate connection's decoding #700
Conversation
2813f21
to
f7417d0
Compare
The decoded function value at each evaluation point. | ||
""" | ||
from nengo.builder.ensemble import get_activities | ||
from nengo.builder.connection import get_targets |
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.
Something was breaking when I put these imports at the top of the file, so I put them here.
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.
Circular imports, I'd guess.
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.
Maybe, but the exception didn't read like that.
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.
¯_(ツ)_/¯
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.
Looked into it... I suppose it's not a circular import at the module level, but it is on the package level... this causes the utils
package to depend on builder
, but builder
already depends on utils
so neither can be imported first. So yeah, I think putting the import here is the right call.
I foresee merge conflicts with #677 ... |
Likely, but I don't think they should be too hard to resolve. I didn't change too much here. I'm more worried about all the other PRs on the stack right now. I forsee a lot of merge conflicts in the near future. |
This LGTM. Needs a changelog entry, and a test if it's not too much trouble. |
Added a changelog entry and a test. |
Made a few changes; give them a look @hunse and I'll merge if they look good to you. As an aside, the Sigmoid neuron type gives interesting results for this! |
LGTM. You're impressed with the sigmoid because the errors are smaller and more regular than for e.g. LIF? |
Not impressed, just interested that it's such a regular pattern. Nothing impresses me anymore... |
Called `utils.connection.eval_point_decoding`. This involved splitting up the build_linear_system helper function so that the eval_points, activities, and targets can all be built separately. In `nengo.utils.ensemble.tuning_curves` we now use the `get_activites` function, which makes it cleaner and fixes a bug where the inputs were being covertly scaled by the radius.
This required a minor change to the builder,
which may break derivative builders.Also fixed a bug in
nengo.utils.ensemble.tuning_curves
to avoid input points being modified.EDIT: I refactored this to keep the
build_linear_system
function, so that nothing should break in other builders.