Skip to content

Fix centered RMSprop: clamp variance, move eps outside sqrt - #73

Closed
sharmis-tp wants to merge 1 commit into
CanReader:masterfrom
sharmis-tp:fix/rmsprop-negative-variance
Closed

sharmis-tp wants to merge 1 commit into
CanReader:masterfrom
sharmis-tp:fix/rmsprop-negative-variance

Conversation

@sharmis-tp

Copy link
Copy Markdown
Contributor

Summary

Fixes #12 — centered RMSprop can take sqrt of a negative value.

  • Clamp the estimated variance to zero before taking the square root, preventing NaN from floating-point cancellation in E[g^2] - E[g]^2 with large gradients (~1e4).
  • Move eps outside the sqrt (sqrt(var) + eps instead of sqrt(var + eps)) to match PyTorch and avoid amplifying steps 10000x when variance is near zero.
  • Replace the test that asserted runaway step growth with one that verifies step-size convergence and finiteness.
  • Add a regression test with 1e4 gradients to guard against the cancellation path.

Test plan

  • cargo test — all 42 unit tests, integration tests, and doc-tests pass
  • New centered_large_gradients_no_nan test feeds 1e4 gradients and asserts no NaN
  • Replaced centered_variant_stabilises_on_constant_gradients verifies convergence

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Centered RMSprop can take the square root of a negative number

2 participants