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.

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

Jump to bottom

SPARK-1215 [MLLIB]: Clustering: Index out of bounds error #1407

Closed
wants to merge 89 commits into from
Closed

SPARK-1215 [MLLIB]: Clustering: Index out of bounds error #1407

wants to merge 89 commits into from

Conversation

jkbradley
Copy link
Member

Bug fix for JIRA SPARK 1215: Clustering: Index out of bounds error

https://issues.apache.org/jira/browse/SPARK-1215

Solution: Print warning, and use duplicate cluster centers so that exactly k centers are returned.

manishamde and others added 30 commits April 20, 2014 13:33
@manishamde
adding support for very deep trees
50b143a
@etrain
Parameterizing max memory.
abc5a23
@manishamde
Merge pull request #5 from etrain/deep_tree
2f6072c 
Parameterizing max memory.
@manishamde
minor: added doc for maxMemory parameter
2f1e093
@etrain
Fixing scalastyle issue.
0287772
@manishamde
Merge pull request #6 from etrain/deep_tree
fecf89a 
Fixing scalastyle issue.
@manishamde
updating user documentation
719d009
@manishamde
merge from master
9dbdabe
@manishamde
updated documentation
1517155
@manishamde
added unit test
718506b
@manishamde
renamed parameter
e0426ee
@manishamde
removed unused imports
dad9652
@manishamde
modified scala.math to math
cbd9f14
@manishamde
added documentation, fixed off by 1 error in max level calculation
5e82202
@manishamde
formatting
4731cda
@manishamde
grammar
5eca9e4
@manishamde
more formatting
8053fed
@manishamde
programming guide blurb
426bb28
@manishamde
formatting
b27ad2c
@manishamde
minor formatting
ce004a1
@manishamde
added docs
7fc9545
@manishamde
merged master
968ca9d
@manishamde
added weighted point class
a1a6e09
@manishamde
changing instance format to weighted labeled point
14aea48
@manishamde
fixed tests
455bea9
@manishamde
todo for multiclass support
46f909c
@manishamde
added multiclass support for find splits bins
4d5f70c
@manishamde
tests for multiclass classification
3f85a17
@manishamde
minor mods
46e06ee
@manishamde
prepared for multiclass without breaking binary classification
6c7af22
mengxr
mengxr reviewed Jul 15, 2014
View reviewed changes
mllib/src/main/scala/org/apache/spark/mllib/clustering/LocalKMeans.scala
if (j == 0) {
logWarning("kMeansPlusPlus initialization ran out of distinct points for centers." +
s" Using duplicate point for center k = $i.")
j = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

The code may be clearer if written in this way

centers(i) = 
  if (j == 0) {
    logWarning("...")
    points(0).toDense
  } else {
    points(j - 1).toDense
  }

or

if (j == 0) {
  logWarning("...")
  centers(i) = points(0).toDense
} else {
  centers(i) = points(j - 1).toDense
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll go with the second suggestion.

@mengxr
Copy link
Contributor

mengxr commented Jul 15, 2014

@jkbradley The fix looks good to me except some minor style issues. Thanks for fixing it! Btw, please add [MLLIB] to the title so this is easy to find.

@SparkQA
Copy link

SparkQA commented Jul 15, 2014

QA tests have started for PR 1407. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16689/consoleFull

@SparkQA
Copy link

SparkQA commented Jul 15, 2014

QA results for PR 1407:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16689/consoleFull

@SparkQA
Copy link

SparkQA commented Jul 16, 2014

QA tests have started for PR 1407. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16712/consoleFull

@jkbradley
Added DecisionTree.print() method for human-readable model descriptio…
eb5700f 
…n. Added temp DTRunnerJKB, eventually to merge with DecisionTreeRunner
@SparkQA
Copy link

SparkQA commented Jul 16, 2014

QA results for PR 1407:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16712/consoleFull

@jkbradley jkbradley changed the title SPARK-1215: Clustering: Index out of bounds error SPARK-1215 [MLLIB]: Clustering: Index out of bounds error Jul 17, 2014
@SparkQA
Copy link

SparkQA commented Jul 17, 2014

QA tests have started for PR 1407. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16785/consoleFull

@SparkQA
Copy link

SparkQA commented Jul 17, 2014

QA results for PR 1407:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16785/consoleFull

@jkbradley jkbradley mentioned this pull request Jul 17, 2014
Closed
@jkbradley
Copy link
Member Author

I tangled stuff in this PR, so I am closing it and resubmitting (with updates per mengxr's suggestions) as PR 1468: #1468

@jkbradley jkbradley closed this Jul 17, 2014
asfgit pushed a commit that referenced this pull request Jul 17, 2014
@jkbradley @mengxr
SPARK-1215 [MLLIB]: Clustering: Index out of bounds error (2)
935fe65 
Added check to LocalKMeans.scala: kMeansPlusPlus initialization to handle case with fewer distinct data points than clusters k.  Added two related unit tests to KMeansSuite.  (Re-submitting PR after tangling commits in PR 1407 #1407 )

Author: Joseph K. Bradley <joseph.kurata.bradley@gmail.com>

Closes #1468 from jkbradley/kmeans-fix and squashes the following commits:

4e9bd1e [Joseph K. Bradley] Updated PR per comments from mengxr
6c7a2ec [Joseph K. Bradley] Added check to LocalKMeans.scala: kMeansPlusPlus initialization to handle case with fewer distinct data points than clusters k.  Added two related unit tests to KMeansSuite.
gzm55 pushed a commit to MediaV/spark that referenced this pull request Jul 18, 2014
@jkbradley
SPARK-1215 [MLLIB]: Clustering: Index out of bounds error (2)
4fac940 
Added check to LocalKMeans.scala: kMeansPlusPlus initialization to handle case with fewer distinct data points than clusters k.  Added two related unit tests to KMeansSuite.  (Re-submitting PR after tangling commits in PR 1407 apache#1407 )

Author: Joseph K. Bradley <joseph.kurata.bradley@gmail.com>

Closes apache#1468 from jkbradley/kmeans-fix and squashes the following commits:

4e9bd1e [Joseph K. Bradley] Updated PR per comments from mengxr
6c7a2ec [Joseph K. Bradley] Added check to LocalKMeans.scala: kMeansPlusPlus initialization to handle case with fewer distinct data points than clusters k.  Added two related unit tests to KMeansSuite.
xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
@jkbradley @conviva-zz
SPARK-1215 [MLLIB]: Clustering: Index out of bounds error (2)
f24062e 
Added check to LocalKMeans.scala: kMeansPlusPlus initialization to handle case with fewer distinct data points than clusters k.  Added two related unit tests to KMeansSuite.  (Re-submitting PR after tangling commits in PR 1407 apache#1407 )

Author: Joseph K. Bradley <joseph.kurata.bradley@gmail.com>

Closes apache#1468 from jkbradley/kmeans-fix and squashes the following commits:

4e9bd1e [Joseph K. Bradley] Updated PR per comments from mengxr
6c7a2ec [Joseph K. Bradley] Added check to LocalKMeans.scala: kMeansPlusPlus initialization to handle case with fewer distinct data points than clusters k.  Added two related unit tests to KMeansSuite.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
7 participants
@jkbradley @AmplabJenkins @mateiz @SparkQA @mengxr @manishamde @etrain