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

[ML] modify validateParams method of org.apache.spark.ml.clustering.LDA.scala file, change docConcentration values from >= 0 to >=1 #11382

Closed
wants to merge 1 commit into from

Conversation

endymecy
Copy link

change 285 and 296 line, modify "getTopicConcentration >= 0" to "getTopicConcentration >= 1"

change 285 and 296 line, modify "getTopicConcentration >= 0" to "getTopicConcentration >= 1"
@endymecy
Copy link
Author

For EM optimizer, docConcentration values must be >= 1,so "getTopicConcentration >= 1" should be right.

@mengxr
Copy link
Contributor

mengxr commented Feb 26, 2016

@endymecy Could you update the PR title to make it more specific?

@mengxr
Copy link
Contributor

mengxr commented Feb 26, 2016

ok to test

@endymecy endymecy changed the title update ml package class LDA.scala For EM optimizer, docConcentration values must be >= 1,so in org.apache.spark.ml.clustering.LDA.scala,should modify validateParams method and change "getTopicConcentration >= 0" to "getTopicConcentration >= 1" Feb 26, 2016
@endymecy endymecy changed the title For EM optimizer, docConcentration values must be >= 1,so in org.apache.spark.ml.clustering.LDA.scala,should modify validateParams method and change "getTopicConcentration >= 0" to "getTopicConcentration >= 1" For EM optimizer, docConcentration values must be >= 1,so in org.apache.spark.ml.clustering.LDA.scala file,should modify validateParams method and change "getTopicConcentration >= 0" to "getTopicConcentration >= 1" Feb 26, 2016
@SparkQA
Copy link

SparkQA commented Feb 26, 2016

Test build #52023 has finished for PR 11382 at commit 0b2723b.

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

@endymecy endymecy changed the title For EM optimizer, docConcentration values must be >= 1,so in org.apache.spark.ml.clustering.LDA.scala file,should modify validateParams method and change "getTopicConcentration >= 0" to "getTopicConcentration >= 1" [ML] modify validateParams method of org.apache.spark.ml.clustering.LDA.scala file, change docConcentration values from >= 0 to >=1 Feb 26, 2016
@srowen
Copy link
Member

srowen commented Feb 26, 2016

@endymecy your title and description are basically a repeat of the diff itself, in English. This is redundant. What is the motivation for the change? that's what you should set out in the description. Normally we'd ask for a JIRA for a functional change.

@jkbradley
Copy link
Member

One more thing: The checks should actually ensure that docConcentration and topicConcentration for EM are > 1.0 (strictly greater). Could you please make that update too?

@rxin
Copy link
Contributor

rxin commented Jun 15, 2016

Thanks for the pull request. I'm going through a list of pull requests to cut them down since the sheer number is breaking some of the tooling we have. Due to lack of activity on this pull request, I'm going to push a commit to close it. Feel free to reopen it or create a new one. We can also continue the discussion on the JIRA ticket.

@asfgit asfgit closed this in 1a33f2e Jun 15, 2016
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.

6 participants