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

ALS implicit: added missing parameter alpha in doc string #3343

Closed
wants to merge 2 commits into from
Closed

ALS implicit: added missing parameter alpha in doc string #3343

wants to merge 2 commits into from

Conversation

felixmaximilian
Copy link
Contributor

No description provided.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@@ -797,6 +797,7 @@ object ALS {
* @param rank number of features to use
* @param iterations number of iterations of ALS (recommended: 10-20)
* @param lambda regularization factor (recommended: 0.01)
* @param alpha confidence parameter (only applies when immplicitPrefs = true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, while we're here, maybe clean a few more things up. "immplicit" is misspelled, and since this is documentation for trainImplicit, there is no implicitPrefs parameter anyway. It's, erm, implicitly true. But these were just true in the text you copied, so can perhaps be fixed elsewhere.

It's really 1 + alpha*R that is the confidence value. alpha sort of controls how difference confidence is in an observed and unobserved interaction. Maybe it's nice to bother to explain a tiny bit more about what this is, in the class scaladoc, and refer to it. While we're here.

The parameter implicitPrefs cannot be set in this context because it is inherent true when calling the trainImplicit method.
@mengxr
Copy link
Contributor

mengxr commented Nov 18, 2014

LGTM. Merged into master and branch-1.2. Thanks!

davies pushed a commit to davies/spark that referenced this pull request Nov 18, 2014
Author: Felix Maximilian Möller <felixmaximilian.moeller@immobilienscout24.de>

Closes apache#3343 from felixmaximilian/fix-documentation and squashes the following commits:

43dcdfb [Felix Maximilian Möller] Removed the information about the switch implicitPrefs. The parameter implicitPrefs cannot be set in this context because it is inherent true when calling the trainImplicit method.
7d172ba [Felix Maximilian Möller] added missing parameter alpha in doc string.
felixmaximilian added a commit that referenced this pull request Nov 19, 2014
Author: Felix Maximilian Möller <felixmaximilian.moeller@immobilienscout24.de>

Closes #3343 from felixmaximilian/fix-documentation and squashes the following commits:

43dcdfb [Felix Maximilian Möller] Removed the information about the switch implicitPrefs. The parameter implicitPrefs cannot be set in this context because it is inherent true when calling the trainImplicit method.
7d172ba [Felix Maximilian Möller] added missing parameter alpha in doc string.

(cherry picked from commit cedc3b5)
Signed-off-by: Xiangrui Meng <meng@databricks.com>
@mengxr
Copy link
Contributor

mengxr commented Nov 21, 2014

@felixmaximilian This was merged but Apache didn't close it automatically. Do you mind closing it? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants