Skip to content

Conversation

@leahmcguire
Copy link
Contributor

No description provided.

@leahmcguire leahmcguire changed the title [SPARK-7545][mllib] Added check in Bernoulli Naive Bayes to make sure that both training and predict feature have values of 0 or 1 [SPARK-7545][mllib] Added check in Bernoulli Naive Bayes to make sure that both training and predict features have values of 0 or 1 May 12, 2015
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Member

Choose a reason for hiding this comment

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

Fix Scala style for multi-line closure, and branch a little less:

if (!brzData.forall(v => v == 0.0 || v == 1.0)) {
  throw new SparkException(
    s"Bernoulli Naive Bayes requires feature values 0 or 1 but found feature vector $testData.")
}

@jkbradley
Copy link
Member

@leahmcguire Thanks for the quick update! I think the 2nd comment is for a bug, but it should be an easy fix.

@mengxr
Copy link
Contributor

mengxr commented May 12, 2015

add to whitelist

@mengxr
Copy link
Contributor

mengxr commented May 12, 2015

test this please

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented May 12, 2015

Test build #32530 has started for PR 6073 at commit 3f3b32c.

@SparkQA
Copy link

SparkQA commented May 12, 2015

Test build #32530 has finished for PR 6073 at commit 3f3b32c.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32530/
Test FAILed.

Copy link
Member

Choose a reason for hiding this comment

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

Could you please move the if (modelType == "Bernoulli") check here so that only one of these require() methods is called?

@jkbradley
Copy link
Member

Looks good other than those 2 minor items. Thank you!

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #32555 has started for PR 6073 at commit 911bf83.

Copy link
Member

Choose a reason for hiding this comment

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

This does need to be called for the Bernoulli model in the createCombiner. Can you please make this an if-else?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry! I meant "does not" but clearly you understood

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #32555 has finished for PR 6073 at commit 911bf83.

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

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32555/
Test PASSed.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #32569 has started for PR 6073 at commit b8442c2.

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #32569 has finished for PR 6073 at commit b8442c2.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32569/
Test FAILed.

@jkbradley
Copy link
Member

spurious failure, retesting

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #804 has started for PR 6073 at commit b8442c2.

@jkbradley
Copy link
Member

LGTM pending tests

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #804 has finished for PR 6073 at commit b8442c2.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #805 has started for PR 6073 at commit b8442c2.

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #805 has finished for PR 6073 at commit b8442c2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class RankingMetrics(JavaModelWrapper):

@jkbradley
Copy link
Member

Merging into master and branch-1.4 @leahmcguire Thank you!

asfgit pushed a commit that referenced this pull request May 13, 2015
…e that both training and predict features have values of 0 or 1

Author: leahmcguire <[email protected]>

Closes #6073 from leahmcguire/binaryCheckNB and squashes the following commits:

b8442c2 [leahmcguire] changed to if else for value checks
911bf83 [leahmcguire] undid reformat
4eedf1e [leahmcguire] moved bernoulli check
9ee9e84 [leahmcguire] fixed style error
3f3b32c [leahmcguire] fixed zero one check so only called in combiner
831fd27 [leahmcguire] got test working
f44bb3c [leahmcguire] removed changes from CV branch
67253f0 [leahmcguire] added check to bernoulli to ensure feature values are zero or one
f191c71 [leahmcguire] fixed name
58d060b [leahmcguire] changed param name and test according to comments
04f0d3c [leahmcguire] Added stats from cross validation as a val in the cross validation model to save them for user access

(cherry picked from commit 61e05fc)
Signed-off-by: Joseph K. Bradley <[email protected]>
@asfgit asfgit closed this in 61e05fc May 13, 2015
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 28, 2015
…e that both training and predict features have values of 0 or 1

Author: leahmcguire <[email protected]>

Closes apache#6073 from leahmcguire/binaryCheckNB and squashes the following commits:

b8442c2 [leahmcguire] changed to if else for value checks
911bf83 [leahmcguire] undid reformat
4eedf1e [leahmcguire] moved bernoulli check
9ee9e84 [leahmcguire] fixed style error
3f3b32c [leahmcguire] fixed zero one check so only called in combiner
831fd27 [leahmcguire] got test working
f44bb3c [leahmcguire] removed changes from CV branch
67253f0 [leahmcguire] added check to bernoulli to ensure feature values are zero or one
f191c71 [leahmcguire] fixed name
58d060b [leahmcguire] changed param name and test according to comments
04f0d3c [leahmcguire] Added stats from cross validation as a val in the cross validation model to save them for user access
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
…e that both training and predict features have values of 0 or 1

Author: leahmcguire <[email protected]>

Closes apache#6073 from leahmcguire/binaryCheckNB and squashes the following commits:

b8442c2 [leahmcguire] changed to if else for value checks
911bf83 [leahmcguire] undid reformat
4eedf1e [leahmcguire] moved bernoulli check
9ee9e84 [leahmcguire] fixed style error
3f3b32c [leahmcguire] fixed zero one check so only called in combiner
831fd27 [leahmcguire] got test working
f44bb3c [leahmcguire] removed changes from CV branch
67253f0 [leahmcguire] added check to bernoulli to ensure feature values are zero or one
f191c71 [leahmcguire] fixed name
58d060b [leahmcguire] changed param name and test according to comments
04f0d3c [leahmcguire] Added stats from cross validation as a val in the cross validation model to save them for user access
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
…e that both training and predict features have values of 0 or 1

Author: leahmcguire <[email protected]>

Closes apache#6073 from leahmcguire/binaryCheckNB and squashes the following commits:

b8442c2 [leahmcguire] changed to if else for value checks
911bf83 [leahmcguire] undid reformat
4eedf1e [leahmcguire] moved bernoulli check
9ee9e84 [leahmcguire] fixed style error
3f3b32c [leahmcguire] fixed zero one check so only called in combiner
831fd27 [leahmcguire] got test working
f44bb3c [leahmcguire] removed changes from CV branch
67253f0 [leahmcguire] added check to bernoulli to ensure feature values are zero or one
f191c71 [leahmcguire] fixed name
58d060b [leahmcguire] changed param name and test according to comments
04f0d3c [leahmcguire] Added stats from cross validation as a val in the cross validation model to save them for user access
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