Thanks for digging into this. You are right, not estimating the means is a problem.
I also think it is strange, and we should deprecate the usage.

So you say matlab uses the same way we currently do it (after the PR)? I looked at vlfeat and they also do the same. I guess it is for numerical reasons. I don't see a strong reason to change it back. Do you?

I don't understand your statement "In #2640, without the snippet resulting the error, I also cannot figure out why the covariance is not positive definite. "
Why would you check the covariance before the cholesky? You could also only raise an error, right?

Besides the deprecation, I don't know why there is init_params and params, which both accept 'wmc' initialization format. It seems redundant.

Yes, I agree with you. I don't think we need to change it back in _covar_mstep_full, but do you think we need to change _covar_mstep_tied? Matlab does as we do now after the PR for full covariance type.

Sorry I don't make that clean. I just don't know why @arendu has non positive definite covariance error in #2640 . I think it would be better to catch numpy.linalg.linalg.LinAlgError: in _log_multivariate_normal_density_full.

I see. I agree it would be nice to catch the error. I thought it was more informative.
I think the intention behind the "duplication" between init_params and params is that you might want to a) give a custom initialization, but want to keep on training (init_params allows that), or want to set some fixed parameters and not train them at all (params allows that).
I think we want to deprecate params because estimating some parameters and not others can lead to trouble.

For init_params it is not something we usually support in scikit-learn, but I can see how it could be useful.
A different way to do this would be to actually pass the initialization as parameters, which is what we do in different places, like means_init=some_array, covars_init=some_array etc.

Yeah. We should provide an explicit way to let users initialize the means, covers, weights with their own initialization. And do you think we need to check users initialize them properly? Such as positive definite covariance, the sum of component weights should be 1.

I will work on the params deprecation and catch numpy.linalg.linalg.LinAlgError:.

Yes, we should check that. I'm +0.5 on changing the way users can initialize. The current way is different from the standard scikit-learn way, but deprecating things is always painful for users.

Thanks for working on this :)

Hello,

Thanks @xuewei4d for the correct explanation of the reason why the covariance matrix was not positive definite.

However, I ran into the problem because I was in a situation where I did want to keep the mean unchanged after initilialization. Basically the idea was to keep the kmeans initialization because it tiles the data cloud while the plain GMM solutions was tending to have confused centers and different covariances to catch non Gaussian tails. I do not claim it is a recommanded way to use GMMs, however, it did the job pretty well.

I think it is convenient to keep 'init_params' and 'params' separate and assume that users know what they do when changing those values. At most, I would add a note in the class documentation to warn that it is an unrecommanded trick.

@alexis-mignon I understand, so let users to worry about how to use init_params and params correctly.

@amueller I will add to my GSoC proposal how to design the interface for passing initialization of mean, covariance, weights in GMM.

thanks :)

@alexis-mignon I think it is not a good idea to keep params, because it is a very special use-case, and we need to make sure that all seven possible combinations work correctly. They did (do?) not, and it looks to me like this is quite a hassle for a very specific use-case.
Also, I'm not even sure what the settings mean. What is the mean without adjusting the prior on the mixture components? Do you just compute the responsibilities and use those to update the mean, but then discard the responsibilities and not update the prior?

I see some interest in being able to keep some parameter fixed:

• You may know the mean values from a phyical model but need to estimate the process noise (using a tied model for instance)
• You may now your noise model, but need to estimate the mean locations (equivalent to deblurring but with discrete samples)

However, I am not sure the corresponding estimators in those settings are exactly the same as the one used in the present EM algorithm. The existence of this paper, , suggests that there might be some specific treatments when some parameters are known. However, I don't have access to science direct and could not read it.

Hurray for closed science!
While I agree that there is an application for that, I would claim it is outside the normal sklearn scope. Sklearn is pretty black-boxy and not really good for plugging in known quantities into models.
I think you'd be better served with a probabilistic modelling framework for that.

What framework would you suggest ?

I'm not sure what the best in Python would be. I'd think of something like factory. I think there is some variational inference machines in python, too. Maybe https://github.com/bayespy/bayespy which also has a list of related packages?

@amueller Besides param problem, I am curious about four covariance type spherical, tied,full,diag, since I never saw that before, at most the case where the covariance is fully estimated, but shared by each component. I found a comment in the gmm code points to a Murphy's 1998 paper, Fitting a Conditional Linear Gaussian Distribution (8 citations). It elaborates all the updating functions for four covariance type. However I didn't find similar material in his MLAPP book. I checked out Matlab implement. There are only two types diag and full, both of them could be shared (tied) or not shared. I think spherical is rarely used. I am just asking is that necessary to refactor covariance updating to that as Matlab does?

spherical is the simplest one, right? I don't see a reason to remove it.
Having tied diagonal might be nice, but no-one ever asked for it, so maybe not so important?

@tguillemot this is fixed in the new GMM, right?

Sorry I haven't seen this issue.
This is fixed by #6666.

Closing this PR based on #4429 (comment). Also the issue is about the original GMM class which is no longer in scikit-learn.