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

Slight error in ADAGrad, momentum should be momentum(i) #155

Open
DanielTakeshi opened this issue Mar 17, 2017 · 0 comments
Open

Slight error in ADAGrad, momentum should be momentum(i) #155

DanielTakeshi opened this issue Mar 17, 2017 · 0 comments

Comments

@DanielTakeshi
Copy link
Contributor

Currently on the master branch, in the ADAGrad code (link to code), in lines 145 and 148, we have:

 ADAGrad.ADAGradm(gmm, gum, gss, momentum.asInstanceOf[GMat], mu, mask.asInstanceOf[GMat], nw.dv.toFloat, gve, gts, glrate, opts.langevin, opts.epsilon, (opts.waitsteps < nsteps));

and

 ADAGrad.ADAGradn(gmm, gum, gss, momentum.asInstanceOf[GMat], mu, mask.asInstanceOf[GMat], nw.dv.toFloat, gve, gts, glrate, opts.langevin, opts.epsilon, (opts.waitsteps < nsteps));

Here, momentum is being cast to a single GMat. However, momentum was defined as an array of GMats at the start of the code. Running ADAGrad with momentum will result in an error since an array can't be cast to a matrix. The fix is to change momentum to momentum(i) for both of these cases, which makes sense since this is in a loop around all the model matrices, and also, the CPU version uses momentum(i).

When I changed it to momentum(i), the GPU version of ADAGrad works correctly.

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

No branches or pull requests

1 participant