Skip to content

[SPARK-4604][MLLIB] make MatrixFactorizationModel public #3459

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.

Sign up for GitHub

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

Closed
mengxr wants to merge 3 commits into from
Closed

[SPARK-4604][MLLIB] make MatrixFactorizationModel public #3459

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a624c12
make MatrixFactorizationModel public
mengxr Nov 25, 2014
ed08214
check preconditions and unit tests
mengxr Nov 26, 2014
f64bcd3
organize imports
mengxr Nov 26, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion mllib/src/main/scala/org/apache/spark/mllib/recommendation/MatrixFactorizationModel.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@ import org.apache.spark.rdd.RDD
/**
* Model representing the result of matrix factorization.
*
* NB: If you create the model directly using constructor, please be aware that fast prediction
Copy link
Member

@jkbradley jkbradley Nov 25, 2014

Choose a reason for hiding this comment

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

What does "NB" mean?

Copy link
Member

@srowen srowen Nov 26, 2014

Choose a reason for hiding this comment

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

NB = "Nota Bene", just "note well"

* requires cached user/product features and the availability of their partitioning information.
Copy link
Member

@jkbradley jkbradley Nov 25, 2014

Choose a reason for hiding this comment

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

It might be good to explain what "availability" means.

*
* @param rank Rank for the features in this model.
* @param userFeatures RDD of tuples where each tuple represents the userId and
* the features computed for this user.
* @param productFeatures RDD of tuples where each tuple represents the productId
* and the features computed for this product.
*/
class MatrixFactorizationModel private[mllib] (
class MatrixFactorizationModel(
Copy link
Member

@jkbradley jkbradley Nov 25, 2014

Choose a reason for hiding this comment

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

With this now public, it might be good to add either (a) one check upon initialization doing a take(1) and comparing with rank, or (b) runtime checks in the various methods in MatrixFactorizationModel. IMO, it's OK if not since those would both add extra overhead, but perhaps there should be a warning for the constructor noting that the arguments are not checked.

Copy link
Contributor Author

@mengxr mengxr Nov 26, 2014

Choose a reason for hiding this comment

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

Done.

val rank: Int,
val userFeatures: RDD[(Int, Array[Double])],
val productFeatures: RDD[(Int, Array[Double])]) extends Serializable {
Expand Down