Raise exception on invalid gains. #1248
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
Seanny123
reviewed
Dec 24, 2016
| @@ -95,6 +96,14 @@ def get_gain_bias(ens, rng=np.random): | |||
| max_rates = get_samples(ens.max_rates, ens.n_neurons, rng=rng) | |||
| intercepts = get_samples(ens.intercepts, ens.n_neurons, rng=rng) | |||
| gain, bias = ens.neuron_type.gain_bias(max_rates, intercepts) | |||
| if gain is not None and ( | |||
| not np.all(np.isfinite(gain)) or np.any(gain <= 0.)): | |||
| raise BuildError( | |||
There was a problem hiding this comment.
Would it be useful to link to a discussion of this problem? Maybe a forum post with the delightful doodles you made?
|
Can we get this PR merged soon? |
tbekolay
force-pushed
the
invalid-intercepts2
branch
from
cbbefa3
to
0c441a1
Compare
tbekolay
approved these changes
Apr 18, 2017
This also requires fixing the calculation of gains and biases for sigmoid neurons. Because the intercepts for sigmoid neurons are actually not an intercept, but the inflection point, valid intercepts depend on whether the max firing rate is below or above the inflection point. This commit also adjusts the default refractory time constant of the sigmoid neurons to play well with intercepts < 1 and max firing rates between 200 and 400Hz. This required some slight adjustments to tests. Fixes #1212.
tbekolay
force-pushed
the
invalid-intercepts2
branch
from
0c441a1
to
ffbb174
Compare
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and context:
As noted in #1212 (and #1231) intercepts >= 1 give unexpected results for most neuron types. The matter is however a bit more complicated as detailed in this comment. The correct generalization to all neuron types (in my opinion) is that non-positive gains give unexpected results. Thus, the core change of this PR is to check this and raise an exception when violated. There is a bunch of related changes, though:
tau_refto 0.0025 which brings the inflection point to 200Hz and the firing rate in the limit to 400Hz. This at least allows to use the sigmoid neuron type without an exception and changing any of the other ensemble defaults. But technically it is a breaking change! We could leavetau_refat it's old value, but would likely need to adjust the max_rates in a lot of our tests.tau_refchanged the sigmoid tuning curves which required to fix a few other tests.Interactions with other PRs:
This PR supersedes #1243.
How has this been tested?
Added some tests for the sigmoid neuron and a test checking that invalid intercepts raise an exception.
How long should this take to review?
Types of changes:
tau_refchange: Bug fix (non-breaking change which fixes an issue)]Checklist: