Skip to content
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

Clean up tests (runtime and failure probability) #118

Merged
merged 7 commits into from Jun 21, 2019

Conversation

@rcurtin
Copy link
Member

commented May 27, 2019

I noticed that the tests took a long time, so I investigated and found that the CNE test can take a really long time. So, I switched it to optimize simpler functions. Then, I checked to make sure that the test failure probability was low, and had to tune a couple tests.

But probably the more interesting thing I did here was that I noticed the BigBatchSGD test sometimes failed after taking a NaN step size. After some investigation, I traced this down to the estimation of the curvature---when the step size gets really small, there is no noticeable different in the iterates, and so arma::norm(iterate - lastIterate, 2) == 0, and the curvature estimate divides by that.

Therefore, the solution I settled on was just to avoid step size decay in this situation by taking the curvature estimate to be 0. @manish7294 I know you were working with BigBatchSGD most recently---let me know what you think of this fix or if something better should be done.

rcurtin added some commits May 27, 2019

Use simpler CNE test functions to save time.
(This can save a *huge* amount of time!  These tests used to take almost 40
minutes.)
Handle case where estimated curvature is NaN.
Also tune the tests a little bit.

@zoq zoq added c: tests t: bugfix and removed s: unlabeled labels May 28, 2019

@rcurtin rcurtin removed the s: unanswered label Jun 5, 2019

@rcurtin rcurtin referenced this pull request Jun 14, 2019
@zoq

zoq approved these changes Jun 19, 2019

Copy link
Member

left a comment

Looks good to me, no comments from my side.

@mlpack-bot

mlpack-bot bot approved these changes Jun 20, 2019

Copy link

left a comment

Second approval provided automatically after 24 hours. 👍

@rcurtin rcurtin merged commit ca92d0e into mlpack:master Jun 21, 2019

2 checks passed

Static Code Analysis Checks Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rcurtin rcurtin deleted the rcurtin:test-cleanup branch Jun 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.