Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The "right" way to calculate the overall covariance for GMM? #72

Closed
sergeant-wizard opened this issue Jan 25, 2017 · 2 comments
Closed

Comments

@sergeant-wizard
Copy link
Contributor

Hi, thanks for publishing this code. It is helping us a lot.

I want to ask about the part where you calculate the overall covariance for GMM.
If my understanding is correct, the overall covariance for GMM with weight w_i is:

codecogseqn

So I think the corresponding change to your current implementation is:

--- a/python/gps/utility/gmm.py
+++ b/python/gps/utility/gmm.py
@@ -89,7 +89,7 @@ class GMM(object):
         # For some reason this version works way better than the "right"
         # one... could we be computing xxt wrong?
         diff = self.mu - np.expand_dims(mu, axis=0)
-        diff_expand = np.expand_dims(diff, axis=1) * \
+        diff_expand = np.expand_dims(self.mu, axis=1) * \
                 np.expand_dims(diff, axis=2)
         wts_expand = np.expand_dims(wts, axis=2)
         sigma = np.sum((self.sigma + diff_expand) * wts_expand, axis=0)

The comment in your code suggests that you're aware that the current code is not theoretically derived, so I'd like to know if my modified version is what you tried as the "right" version, and if so, how it performed "worse" compared to the current version.

I did try the above modification with mcj_badmm_example, and I couldn't find any significant difference between the original version.

Thanks!

@justinjfu
Copy link
Contributor

Hi,

We're aware that the code is mathematically incorrect. If you would like, you can make a PR and we can run the benchmark tasks to make sure there is no performance degradation.

We've been planning on replacing this GMM with a version based on the scikit-learn implementation, so this code will likely be removed in the future.

@sergeant-wizard
Copy link
Contributor Author

Thanks for the response.
I submitted a PR as #73 if only for the reason that I get to see the benchmark results.
Look forward to hearing from you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants