merge Autogmm and GaussianCluster

[bdpedigo]

Seems like there is enough redundant code, they are both doing something similar, and we can still get both behaviors with different options

[bdpedigo]

to clarify, AutoGMM does everything that GaussianCluster does and more. The difference is, GaussianCluster has an option to run kmeans initialization multiple times, which I have found empirically can lead to better results than any of the AutoGMM initializations. If AutoGMM had a way of running multiple kmeans inits and taking the best, we could deprecate GaussianCluster

[CaseyWeiner]

I'll get this done this semester

[bdpedigo]

https://arxiv.org/abs/1909.02688

[PerifanosPrometheus]

@bdpedigo I will take this one, sounds fun! : )

DoD:

• Implementation:
  • Add option in AutoGMM class to run k-means multiple times:
    • Must be simple and intuitive to call for the user
    • Must be able to specify min_components and max_components to use
      - If max_components is None then it must utilize min_components as maximum number(analogously to gclust)
    • Must be integrated with the k-means method already present in the AutoGMM class
• Testing:
  • Must have identical output as gclust. Meaning that the difference between number of clusters assigned by gclust and the merged functionality to autoGMM must be identically 0 given same input data and same settings

This is an initial very much qualitative assesment of what the final implementation should do. I am sure it will progressively become more and more quantitative.

Please let me know if you have any suggestions/ideas.

[PerifanosPrometheus]

https://scikit-learn.org/stable/modules/generated/sklearn.linear_model.LassoLarsIC.html

[bdpedigo]

see criterion_ for example
criterion_: array-like of shape (n_alphas,) The value of the information criteria (‘aic’, ‘bic’) across all alphas. The alpha which has the smallest information criterion is chosen. This value is larger by a factor of n_samples compared to Eqns. 2.15 and 2.16 in (Zou et al, 2007).

[bdpedigo]

@PerifanosPrometheus: @tliu68 is working on some changes to the internals of AutoGMM (see #371). I think this will just affect the initialization. You two might want to coordinate a bit so that the merge is easy at the end!

[bdpedigo]

@tliu68 this PR from @PerifanosPrometheus will more or less add 1 or more runs of kmeans as an initialization in AutoGMM, and maybe will rename the class itself.

[tliu68]

@tliu68 this PR from @PerifanosPrometheus will more or less add 1 or more runs of kmeans as an initialization in AutoGMM, and maybe will rename the class itself.

Cool! @PerifanosPrometheus I'll let you know what specific changes I'm going to make soon. Thanks!

[PerifanosPrometheus]

@tliu68 this PR from @PerifanosPrometheus will more or less add 1 or more runs of kmeans as an initialization in AutoGMM, and maybe will rename the class itself.

Cool! @PerifanosPrometheus I'll let you know what specific changes I'm going to make soon. Thanks!

@tliu68 Currently, I am also on the planning stage so I haven't finalized any changes yet. Please let me know if there is anything that I should particularly be careful dealing with

[tliu68]

@tliu68 Currently, I am also on the planning stage so I haven't finalized any changes yet. Please let me know if there is anything that I should particularly be careful dealing with

@PerifanosPrometheus I've opened a draft PR #589. Please see my description on all major changes I made. My plan for next step is to try to clean up/reorganize the for loops and see if I can incorporate parallel processing. But I believe the major updates we wanted to make have been addressed. Please let me know if you have any question and all suggestion/advice welcome!

[jovo]

@tliu68 @PerifanosPrometheus please coordinate

[PerifanosPrometheus]

@tliu68 I would like to continue working on this issue, however I noticed that PR #589 has been merged. I would like to touch back with you to have an update on changes merged.

Mainly, I believe that fixing this issue will involve modifying the param_grid dict in the previous implementation, has that been changed?

Lastly, I heard that you were planning on merging the function with sklearn(not entirely sure this info is correct) in that case what would be the timeline to make my changes(if even desirable).

[tliu68]

@tliu68 I would like to continue working on this issue, however I noticed that PR #589 has been merged. I would like to touch back with you to have an update on changes merged.

Mainly, I believe that fixing this issue will involve modifying the param_grid dict in the previous implementation, has that been changed?

Lastly, I heard that you were planning on merging the function with sklearn(not entirely sure this info is correct) in that case what would be the timeline to make my changes(if even desirable).

Yes, I am working on merging this into sklearn (described in issue #601). The previous PR #589 basically changed the initialization process so that instead of running AgglomerativeClustering and GaussianMixture for all combinations in the old param_grid in parallel, now

• for each set of agglomerative parameters,
• AgglomerativeClustering is run for the smallest n_components (which is why I added another output param_grid_ag from _process_paramgrid -- to separate out the agglomerative parameters)
• and a hierarchy of clusters is generated based on that assignment,
• then after calculating all agglomerative clustering assignments, GaussianMixture is run for all combinations in param_grid

Does that make sense? Also, Ben and I will probably make more changes (to be discussed) but it would be great if we can start a PR into sklearn soon. Do you mind sharing your plan?