[SYSTEMDS-3926] add test nn optimizers#2538
Open
Darius3001 wants to merge 31 commits into
Open
Conversation
Add previously-missing invariant checks flagged in code review: adam.dml - inline lr_eff for the 1xN shape case (stop piggy-backing on lr_eff1 from the 2x3 case; a shape bug would have silently passed) - sign / direction sanity: at X=0 with m=v=0 the step is opposite to dX - state independence from X: m,v depend only on dX and prior state - textbook vs compact bias-correction equivalence at t=2 (1e-4 slack for the epsilon-placement difference) - lr=0 no-op (parameters must not move) - large-gradient (1e10) finiteness and step-magnitude bound - no-stall check on the convergence loop (loss at step 400 < loss at 200) adamw.dml - lambda=0 reduces AdamW to Adam (X within 1e-4 for eps placement, m/v exactly to 1e-12) — the strongest single decoupling invariant - sign / direction sanity (lambda=0 pins the adaptive term's sign) - state independence from X - lr=0 no-op with lambda>0 (both terms scale with lr; X must be pinned) - large-gradient finiteness with a decay-inclusive bound - decay-heavy convergence run (lambda=0.1) — the existing lambda=0.001 run was basically Adam - clarify the t=1 collapse in the mixed-gradient analytic comment adagrad.dml - replace magic min(cache)<10 threshold with an anchored 2x growth check over the observed step-1 cache - non-negativity of cache (single- and multi-step) - mixed-sign accumulation: c += dX^2 must not depend on sign of dX - sign / direction sanity - lr=0 no-op (parameters pinned, cache still accumulates) - large-gradient finiteness and step ~lr bound Mutation-tested: a sign flip in adam.dml, a cache-reset and a cache+=|dX| mutation in adagrad.dml, and dropping *X from AdamW's decay term or using beta*t instead of beta^t all now surface as ERROR: lines from the new assertions above.
test: close review gaps in adam/adamw/adagrad optimizer tests
Darius3001
changed the title
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
3 participants
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.
This PR adds tests for optimizers + verifies that new optimizers work out of the box [SYSTEMDS-3926] .
We have added a lot of tests for each optimizer + added a test suite/framework where you can input an optimizer which would then be used inside a neural net training with MNIST handwritten numbers dataset.
I've added instructions on how to add optimizers to the testsuite. In the future you can also input different arguments here to be able to test different parameters for the optimizers.
In the test suite it is tested that the accuracy increases each epoch and loss function decreases