[MRG+1] Adding a fit_predict method for the GMM#4593
[MRG+1] Adding a fit_predict method for the GMM#4593amueller merged 1 commit intoscikit-learn:masterfrom
Conversation
sklearn/mixture/gmm.py
Outdated
There was a problem hiding this comment.
It's better to use the _check_fitted_model: do a git grep _check_fitted_model to see examples in the code base.
There was a problem hiding this comment.
actually this comment is no longer relevant in light of the other comments.
9349c48 to
18d4df4
Compare
sklearn/mixture/gmm.py
Outdated
There was a problem hiding this comment.
I would return directly:
return self._fit(X, y).argmax(axis=1)and make sure that _fit can never return None (make it raise a ValueError or similar with a meaningful error message otherwise).
There was a problem hiding this comment.
It could happen in the case when n_iter == 0. You're right that never returning None is better, so I added a check to run score_samples to get the correct output value in that case (that apparently happens when running an HMM). My current idea is just to output zeros because the idea of n_iter=0 seems to be to quickly initialize a model.
|
I am not sure I understand the travis failures, it probably requires to launch a debugger. |
18d4df4 to
986defe
Compare
|
@ogrisel Besides addressing your comments, I changed the GMM subclass |
|
For the travis failure, a solution would be to not implement
|
|
Ah alright, ignore my last comment, I had an internet connection pbm and could not post it when I first wrote it. Now I see that fixed the problems with the subclasses. |
There was a problem hiding this comment.
Great! Thanks for having updated that test.
986defe to
234bd07
Compare
|
@ogrisel Added what's new. Thanks for all your help! |
234bd07 to
ff4d8d2
Compare
There was a problem hiding this comment.
It needs to document its return value.
With low iterations, the prediction might not be 100% accurate due to
the final maximization step in the EM algorithm.
See issue:
#4579