Modified GMM initialization to only use linear memory and time in sph… #6720
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…erical and diag cases
|
The GMM class is now deprecated and replaced by the new GaussianMixture mixture. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issue
Not an reported issue, to my knowledge.
What does this implement/fix? Explain your changes.
When running a 'diag' GMM on a high-dimensional feature space, a Memory Error was thrown. This is suspicious because quadratic memory should not be used for diagonal GMM fitting. Indeed, there was a section of gmm.py that had a needless covariance matrix calculation for the 'diag' and 'spherical' cases.
The modified code fixes that; behavior should be the same for 'diag'.
For 'spherical', the previous code initialized the variance term to the average value of all terms in the covariance matrix. The new code CHANGES this behavior; now it is the average value of the diagonal terms only. This is a more appropriate initialization anyway.
Any other comments?