I have not make KDE to be based on DensityMixin. @ogrisel @lesteve @amueller .

The thing with changing score_samples is that it is an incompatible change. So we have to a) accept that we break user code (rather not) or b) rename the estimator to GaussianMixtureModel or something, and make the old class behave like it did, deprecate it, and make the new class behave as we want.

Sorry for my bone-headness. I will build a new Class GaussianMixtureModel. And we should add deprecate messages in score_samples right?

Rename the old class to the new name, then implement GMM by inheriting from the new class and overwriting score_samples. Then deprecate the whole "old" GMM. At least I think that was the consensus.

Thanks for the advice. Did you mean to rename class GMM to class GaussianMixtureModel and implement GMM in the same file, i.e. gmm.py?

yes, that sounds good.

I am sorry I didn't get a very clear mind in the beginning, and there is a lot of work on refactoring GMM. So I removed the GaussianMixtureModel I created for the inconsistency problem about score, but I created a new class in an separated file, just an prototype. Here is that my thought.

• added the parameters weights, means, covars in __init__ to allow users initialize the model
• added methods to check user-provided initialization data
• added methods that compute the sufficient statistics of the data, which is used in m-step. They are N_k, \bar{x}_k, S_k.
• added estimation functions as abstract methods to the base class _MixtureModelBase such that GaussianMixtureModel, BayesianMixtureModel, DirichletProcessMixtureModel could implement their own updating functions.
• added DensityMixin to compute score for all density estimators

The code is in the earliest version. I would like to hear about the comments about the definitions of these classes. Thanks.

I may be a bit late to the party but if these classes get renamed, may I object to having "Model" in the name, and suggest simply GaussianMixture etc? I mean, most of our estimators are models of some sort, the word doesn't bring much, and the docstrings will still say "Gaussian Mixture Model" and "GMM" for searchability.

@vene from my perspective, the name 'GaussianMixtureModel' is OK, because in the literature everybody uses this name. The word 'model' is kind of a part of it.

I am confused with the recent PR #4593 @clorenz7 . How can zero-filled responsibility is a kind of initialization? Line571. The L1-norm of each responsibility must be 1. If we does need quick-initialization, we should initialize the model parameters, weights_, means_, covariances_, not responsibility.

self.n_iter == 0 occurs when using GMM within HMM

        # Need to make sure that there are responsibilities to output
        # Output zeros because it was just a quick initialization

I cannot find somewhere needs responsibilities, except self.fit_predict merged from the PR #4593.
HMM of sklearn is gone. hmmlearn does not set n_iter=0. _fit is a private method, which should not be called by other modules. So returning responsibility is only for self.fit_predict. However, this method directly use the responsibility of _fit, which will skip the last EM iteration, because the returned responsibility is computed with parameters optimized by the last two iteration.

And Line442 has a weird parameter do_prediction=False??

Yes, do_prediction is not needed, I apologize for my oversight.

There was discussion in the PR pre-merge about the removal of HMM, and the
not-needed n_iter=0. It is not that another module would be calling
self._fit, but that a module which subclassed from it would make use of it.
The addition was largely defensive since responsibilities variable was set
in the for loop over n_iter. The responsibilities being all zeros as a
degenerate case seemed ok since a GMM fit with n_iter=0 is already
degenerate.

You are right about the responsibility returning only being for
fit_predict, and it skipping the last EM iteration. I wrote a comment about
that:

Warning: due to the final maximization step in the EM algorithm, with low
iterations the prediction may not be 100% accurate ```
During the development of the method, the feedback that I received was that
that the idea of the fit_predict method is to save time over someone just
calling ```model.fit``` then ```model.predict```.







On Wed, Jun 17, 2015 at 11:10 AM, Wei Xue <notifications@github.com> wrote:

> I am confused with the recent PR #4579
> <https://github.com/scikit-learn/scikit-learn/issues/4579> @clorenz7
> <https://github.com/clorenz7> . How can zero-filled responsibility is a
> kind of initialization? Line571
> <https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/mixture/gmm.py#L571>.
> The L1-norm of each responsibility must be 1. If we does need
> quick-initialization, we should initialize the model parameters, weights_,
> means_, covariances_, not responsibility.
>
> # self.n_iter == 0 occurs when using GMM within HMM
> # Need to make sure that there are responsibilities to output
> # Output zeros because it was just a quick initialization
>
> I cannot find somewhere needs responsibilities, except self.fit_predict
> merged from the PR #4579
> <https://github.com/scikit-learn/scikit-learn/issues/4579>.
> HMM of sklearn is gone. hmmlearn does not set n_iter=0
> <https://github.com/hmmlearn/hmmlearn/blob/master/hmmlearn/hmm.py#L592>.
> _fit is a private method, which should not be called by other modules. So
> returning responsibility is only for self.fit_predict. However, this
> method directly use the responsibility of _fit, which will skip the last
> EM iteration, because the returned responsibility is computed with
> parameters optimized by the last two iteration.
>
> And Line442
> <https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/mixture/gmm.py#L442>
> has a weird parameter do_prediction=False??
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/scikit-learn/scikit-learn/pull/4802#issuecomment-112896427>
> .
>

+1 for simplifying the code by taking the removal of HMM into account and not dealing with the n_iter=0 case that should never appear.

#5344 might be of interest

Just renamed the file names, fixed relative import issue, and fixed PEP8 problems in test files

Could you rebase? :)

I've looked through the current documentation (and derivation PDF) and I think it's missing the explanation of different covariance types.

We had a similar issue in hmmlearn recently, so feel free to use the explanation in hmmlearn/hmmlearn#60.

@superbobry Thanks!

@xuewei4d did you get the opportunity to understand what's wrong with the gamma_prior of your implementation of DP mixture models not working as expected (no impact on the number of components)?

In your notebook, you should seed the random_state=0 parameter of the model to make different runs easier to compare. Also the debug snapshot should call .copy() on all the arrays it snapshot. However this is not the cause of the problem.

I just come out the possible reason for the lower bound of VBGMM goes up and down. In VBGMM and DPGMM, some clusters may vanish during fitting, so the actual the number of clusters would not be the parameter self.n_components. Maybe the value should be adjusted when some clusters are gone. I not quite sure. What I mean 'the clusters are gone' is that these clusters do not have enough data points to make its covariance positive definite. @tguillemot

@xuewei4d in my experience oscillating lower bound almost always means a bug in the M-step (or posterior update in case of VB-type algorithms).

What I mean 'the clusters are gone' is that these clusters do not have enough data points to make its covariance positive definite.

Wouldn't the covariance be completely defined by the prior in this case?

This might actually provide a good reference: https://github.com/teodor-moldovan/dpcluster

@amueller Thanks I will have a look to that.

Closing in favor of #7295. Let's finish the review there.