Skip to content

[SPARK-4894][mllib] Added Bernoulli option to NaiveBayes model in mllib #4087

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
leahmcguire wants to merge 31 commits into from
Closed

[SPARK-4894][mllib] Added Bernoulli option to NaiveBayes model in mllib #4087

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
ce73c63
added Bernoulli option to niave bayes model in mllib, added optional …
leahmcguire Jan 16, 2015
4a3676d
Updated changes re-comments. Got rid of verbose populateMatrix method…
leahmcguire Jan 21, 2015
0313c0c
fixed style error in NaiveBayes.scala
leahmcguire Jan 21, 2015
76e5b0f
removed unnecessary sort from test
leahmcguire Jan 26, 2015
d9477ed
removed old inaccurate comment from test suite for mllib naive bayes
leahmcguire Feb 26, 2015
3891bf2
synced with apache spark and resolved merge conflict
leahmcguire Feb 27, 2015
5a4a534
fixed scala style error in NaiveBayes
leahmcguire Feb 27, 2015
b61b5e2
added back compatable constructor to NaiveBayesModel to fix MIMA test…
leahmcguire Mar 2, 2015
3730572
modified NB model type to be more Java-friendly
jkbradley Mar 3, 2015
b93aaf6
Merge pull request #1 from jkbradley/nb-model-type
leahmcguire Mar 5, 2015
7622b0c
added comments and fixed style as per rb
leahmcguire Mar 5, 2015
dc65374
integrated model type fix
leahmcguire Mar 5, 2015
85f298f
Merge remote-tracking branch 'upstream/master'
leahmcguire Mar 5, 2015
e016569
updated test suite with model type fix
leahmcguire Mar 5, 2015
ea09b28
Merge remote-tracking branch 'upstream/master'
leahmcguire Mar 5, 2015
900b586
fixed model call so that uses type argument
leahmcguire Mar 5, 2015
b85b0c9
Merge remote-tracking branch 'upstream/master'
leahmcguire Mar 5, 2015
c298e78
fixed scala style errors
leahmcguire Mar 5, 2015
2d0c1ba
fixed typo in NaiveBayes
leahmcguire Mar 5, 2015
e2d925e
fixed nonserializable error that was causing naivebayes test failures
leahmcguire Mar 7, 2015
fb0a5c7
removed typo
leahmcguire Mar 9, 2015
01baad7
made fixes from code review
leahmcguire Mar 11, 2015
bea62af
put back in constructor for NaiveBayes
leahmcguire Mar 12, 2015
18f3219
removed private from naive bayes constructor for lambda only
leahmcguire Mar 12, 2015
a22d670
changed NaiveBayesModel modelType parameter back to NaiveBayes.ModelT…
leahmcguire Mar 17, 2015
852a727
merged with upstream master
leahmcguire Mar 21, 2015
6a8f383
Added new model save/load format 2.0 for NaiveBayesModel after modelT…
jkbradley Mar 22, 2015
9ad89ca
removed old code
jkbradley Mar 22, 2015
2224b15
Merge pull request #2 from jkbradley/leahmcguire-master
leahmcguire Mar 24, 2015
acb69af
removed enum type and replaces all modelType parameters with strings
leahmcguire Mar 28, 2015
f3c8994
changed checks on model type to requires
leahmcguire Mar 31, 2015
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
Prev Previous commit
Next Next commit
fixed model call so that uses type argument
  • Loading branch information
@leahmcguire
leahmcguire committed Mar 5, 2015
commit 900b5864c16cc0db93a46ec3a4591a787e5a21a0
8 changes: 4 additions & 4 deletions mllib/src/main/scala/org/apache/spark/mllib/classification/NaiveBayes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,21 +310,21 @@ object NaiveBayes {
*
* The model type can be set to either Multinomial NB ([[http://tinyurl.com/lsdw6p]])
* or Bernoulli NB ([[http://tinyurl.com/p7c96j6]]). The Multinomial NB can handle
* discrete count data and can be called by setting the model type to "Multinomial".
* discrete count data and can be called by setting the model type to "multinomial".
* For example, it can be used with word counts or TF_IDF vectors of documents.
* The Bernoulli model fits presence or absence (0-1) counts. By making every vector a
* 0-1 vector and setting the model type to "Bernoulli", the fits and predicts as
* 0-1 vector and setting the model type to "bernoulli", the fits and predicts as
* Bernoulli NB.
*
* @param input RDD of `(label, array of features)` pairs. Every vector should be a frequency
* vector or a count vector.
* @param lambda The smoothing parameter
*
* @param modelType The type of NB model to fit from the enumeration NaiveBayesModels, can be
* Multinomial or Bernoulli
* multinomial or bernoulli
*/
def train(input: RDD[LabeledPoint], lambda: Double, modelType: String): NaiveBayesModel = {
Copy link
Member

@jkbradley jkbradley Mar 10, 2015

Choose a reason for hiding this comment

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

I'm actually wondering if we even need to provide this static train method. Users can use the builder pattern new NaiveBayes().setLambda().setModelType().run() instead. Could you please remove it? Sorry for not noticing this earlier!

Copy link
Contributor Author

@leahmcguire leahmcguire Mar 11, 2015

Choose a reason for hiding this comment

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

If we remove this static train method should we also remove the static train method that just includes lambda (line 326). Otherwise the train calls are inconsistent for setting different model parameters (lambda and modelType).

Copy link
Member

@jkbradley jkbradley Mar 14, 2015

Choose a reason for hiding this comment

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

I wouldn't call it inconsistent, but it does limit users who want static methods. Adding new train() methods for every new parameter ended up hurting us for trees & ensembles because the list became so long, so I'd still vote for removing this static train method. We could even deprecate the other static train method and tell users to use the builder pattern instead. If you feel strongly about it, though, I'm OK with leaving it as is (since there are not very many parameters for NB).

new NaiveBayes(lambda, Multinomial).run(input)
new NaiveBayes(lambda, MODELTYPE.fromString(modelType)).run(input)
}


Expand Down
4 changes: 2 additions & 2 deletions mllib/src/test/scala/org/apache/spark/mllib/classification/NaiveBayesSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class NaiveBayesSuite extends FunSuite with MLlibTestSparkContext {
val testRDD = sc.parallelize(testData, 2)
testRDD.cache()

val model = NaiveBayes.train(testRDD, 1.0, "Multinomial")
val model = NaiveBayes.train(testRDD, 1.0, "multinomial")
validateModelFit(pi, theta, model)

val validationData = NaiveBayesSuite.generateNaiveBayesInput(
Expand Down Expand Up @@ -161,7 +161,7 @@ class NaiveBayesSuite extends FunSuite with MLlibTestSparkContext {
val testRDD = sc.parallelize(testData, 2)
testRDD.cache()

val model = NaiveBayes.train(testRDD, 1.0, "Bernoulli")
val model = NaiveBayes.train(testRDD, 1.0, "bernoulli")
validateModelFit(pi, theta, model)

val validationData = NaiveBayesSuite.generateNaiveBayesInput(
Expand Down