Skip to content

Conversation

@MLnick
Copy link
Contributor

@MLnick MLnick commented Feb 28, 2017

SPARK-14489 added the ability to skip NaN predictions during ALSModel.transform. This PR adds documentation for the coldStartStrategy param to the ALS user guide, and add code to the examples to illustrate usage.

How was this patch tested?

Doc and example change only. Build HTML doc locally and verified example code builds, and runs in shell for Scala/Python.

@SparkQA
Copy link

SparkQA commented Feb 28, 2017

Test build #73599 has finished for PR 17102 at commit 4c2c78c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

@holdenk holdenk left a comment

Choose a reason for hiding this comment

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

Python side looks good :)

Copy link
Contributor

@sethah sethah left a comment

Choose a reason for hiding this comment

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

A couple really small things. Otherwise the changes look good. I didn't build the docs or run the examples.

The evaluation metric will then be computed over the non-`NaN` data and will be valid.
Usage of this parameter is illustrated in the example below.

**Note:** currently the supported cold start strategies are `nan` (the default behavior mentioned
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit wary of putting the options explicitly here, but it seems hard to avoid since they're mentioned above. Even so, maybe use "drop" and "nan" (quotes).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah here I wanted to explicitly mention the "drop" option. Ideally will remove this note section when further strategies are added (like the average user vector idea).

scenarios:

1. In production, for new users or items that have no rating history and on which the model has not
been trained (this is the "cold start problem")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add punctuation (other places in the user guide have punctuation despite the fact that we are listing things)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure thing

@SparkQA
Copy link

SparkQA commented Mar 1, 2017

Test build #73703 has finished for PR 17102 at commit c422d58.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MLnick
Copy link
Contributor Author

MLnick commented Mar 2, 2017

Merged to master

@asfgit asfgit closed this in 9cca3db Mar 2, 2017
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

Successfully merging this pull request may close these issues.

5 participants