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

Implemeted gradient descent optimizer. #792

Merged
merged 4 commits into from Oct 7, 2016

Conversation

Projects
None yet
2 participants
@sumedhghaisas
Member

sumedhghaisas commented Sep 27, 2016

No description provided.

Show outdated Hide outdated src/mlpack/core/optimizers/gradient_descent/gradient_descent_impl.hpp
double lastObjective = DBL_MAX;
// Now iterate!
arma::vec gradient(iterate.n_cols);

This comment has been minimized.

@rcurtin

rcurtin Sep 29, 2016

Member

This should be arma::mat not arma::vec, and you should use both the number of rows and columns to initialize it. Otherwise it will fail if we use an objective function that uses a 2-d matrix instead of a 1-d vector.

@rcurtin

rcurtin Sep 29, 2016

Member

This should be arma::mat not arma::vec, and you should use both the number of rows and columns to initialize it. Otherwise it will fail if we use an objective function that uses a 2-d matrix instead of a 1-d vector.

Show outdated Hide outdated src/mlpack/core/optimizers/gradient_descent/gradient_descent_impl.hpp
{
Log::Warn << "Gradient Descent: converged to " << overallObjective
<< "; terminating" << " with failure. Try a smaller step size?"
<< std::endl;

This comment has been minimized.

@rcurtin

rcurtin Sep 29, 2016

Member

Continued lines should be intended two tabs from the previous line, not one :)

@rcurtin

rcurtin Sep 29, 2016

Member

Continued lines should be intended two tabs from the previous line, not one :)

@rcurtin

This comment has been minimized.

Show comment
Hide comment
@rcurtin

rcurtin Sep 30, 2016

Member

Once you fix the constructor to arma::mat so it compiles I think this is ready to merge. :)

Member

rcurtin commented Sep 30, 2016

Once you fix the constructor to arma::mat so it compiles I think this is ready to merge. :)

@rcurtin

This comment has been minimized.

Show comment
Hide comment
@rcurtin

rcurtin Oct 2, 2016

Member

Ah I forgot, we should also add something to HISTORY.md to point out that we now have a gradient descent optimizer. :)

Member

rcurtin commented Oct 2, 2016

Ah I forgot, we should also add something to HISTORY.md to point out that we now have a gradient descent optimizer. :)

@rcurtin rcurtin merged commit 5fd5395 into mlpack:master Oct 7, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment