Keep gradients finite when electrodes are driven to zero amplitude - #4
Open
charliefay38221-sudo wants to merge 1 commit into
Conversation
The default sqrt size equation (Tehovnik 2007) computes
sqrt(x / current_spread)
whose derivative 1/(2*sqrt(x)) is infinite at x = 0. An electrode driven to
exactly zero amplitude therefore emits a NaN gradient that propagates into
every parameter of the model, while the forward pass continues to look
correct. Nothing signals the corruption until the whole network is NaN, which
presents as a diverged model rather than a bug.
This is reachable in ordinary use rather than a corner case: sparse
stimulation is the goal for a charge-limited device, sigmoid encoder outputs
underflow to zero, and any charge penalty drives amplitudes down. With one
electrode in four at zero, 25% of the amplitude gradients are NaN.
Clamping the argument keeps the derivative finite. The bound is far below any
physical stimulation current, so the forward output is unchanged: the worst
absolute difference over 30 random varied-amplitude batches is exactly zero.
The sigmoid size equation is smooth at zero and is unaffected.
Adds a regression test covering 100%, 50% and 25% of electrodes off, plus a
check that the forward percept is untouched. The gradient tests fail on the
previous behaviour and the forward test passes either way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Sigma's default size equation issqrt(x / current_spread). The derivative ofsqrtis infinite at zero, so an electrode driven to exactly 0 A emits a NaNgradient that propagates into every parameter, while the forward pass still
looks correct. With one electrode in four at zero, 25% of the amplitude
gradients are NaN.
This is reachable in ordinary use: sparse stimulation is the goal for a
charge-limited device, sigmoid encoder outputs underflow to zero, and a charge
penalty drives amplitudes there. It presents as training diverging rather than
as a bug, because nothing is visibly wrong until the whole model is NaN.
Both other implementations of this model already guard against it:
SafetyLayer.forwardends.clamp(1e-32, None)DynaphosModeldoessigma = np.where(amp > 0, np.clip(P / 2, 1e-22, None), sigma),commented "only update sigma if amplitude > 0"
This clamps the argument instead. Forward output is unchanged: worst
absolute difference exactly 0.000e+00 over 30 random varied-amplitude batches.
The
sigmoidsize equation was never affected.Adds a regression test covering 100%, 50% and 25% of electrodes off, plus a
check that the forward percept is untouched. Mutation tested: with the clamp
removed the three gradient cases fail and the forward case still passes, so the
suite is detecting the defect rather than merely detecting change.
Scope note: this fixes NaN gradients only. It does not by itself let an encoder
that has drifted below rheobase recover, since
reluinget_currentand thegreatervisibility gate are separately flat there. Happy to raise thatseparately if it is of interest.