Samanthacampo/kmeans++ #34
Conversation
Clustering/KMeans/src/main/java/org/tribuo/clustering/kmeans/KMeansTrainer.java
Show resolved
Hide resolved
| @@ -259,17 +286,16 @@ public int getInvocationCount() { | |||
| } | |||
|
|
|||
| /** | |||
| * Initialisation method called at the start of each train call. | |||
| * Initialisation method called at the start of each train call when | |||
| * using uniform centroid initialisation. | |||
There was a problem hiding this comment.
I think this should say "Centroids are initialised using a uniform random sample from the feature domain" somewhere in the javadoc.
|
|
||
| // go through every vector and see if the min distance to the | ||
| // newest centroid is smaller than previous min distance for vec | ||
| double tempDistance; |
There was a problem hiding this comment.
There's no need to define tempDistance outside the for loop. Better to keep it inside so the compiler doesn't think it actually lives any longer.
Clustering/KMeans/src/main/java/org/tribuo/clustering/kmeans/KMeansTrainer.java
Show resolved
Hide resolved
| @@ -54,6 +55,9 @@ public String getOptionsDescription() { | |||
| public int iterations = 10; | |||
| @Option(charName='d',longName="distance",usage="Distance function to use in the e step. Defaults to EUCLIDEAN.") | |||
| public Distance distance = Distance.EUCLIDEAN; | |||
| @Option(charName='i',longName="initialisation",usage="Type of initialisation to use for centroids. Defaults " + | |||
There was a problem hiding this comment.
There already is a i argument, so this options class is invalid, could you change it to something else?
There was a problem hiding this comment.
Also, there's no need to put the default into the usage string, I updated the options code to print that automatically. While you're in here could you remove the default bit from the other usage strings? I missed that in the docs pass I did.
Add kmeans++ centroid initialization option to kmeans.