Skip to content

Add a comment#14242

Closed
kzhang28 wants to merge 2 commits intoapache:masterfrom
kzhang28:master
Closed

Add a comment#14242
kzhang28 wants to merge 2 commits intoapache:masterfrom
kzhang28:master

Conversation

@kzhang28
Copy link

What changes were proposed in this pull request?

(comment added, no source code changed)

How was this patch tested?

(unit test)

Please make sure the Kmeans input dataset does not contain duplicates, otherwise takeSample() method may take duplicated examples, leading to runtime exception

kzhang28 added 2 commits July 17, 2016 19:03
Please make sure the Kmeans input dataset does not contain duplicates, otherwise takeSample() method may take duplicated examples.
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

val convergeDist = args(2).toDouble

/*
* A requirement of using takeSample() method: the element in data (line 74) should be unique (No duplicates), otherwise the size of newPoints (line 89) will not
Copy link
Member

Choose a reason for hiding this comment

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

This sounds like a comment for a caller, but it's not visible to anyone but those reading source code.

Why not just call .distinct in the code to avoid the problem entirely?

@srowen
Copy link
Member

srowen commented Jul 23, 2016

@kzhang28 update or close this?

@kzhang28 kzhang28 closed this Jul 23, 2016
@kzhang28
Copy link
Author

@srowen I closed it. Thank you for your kind reminder.

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.

4 participants