-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Adjusted Mutual Information #402
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
Conversation
…d on this to come!)
…m in the morning.
Thanks for this contrib! |
|
||
|
||
def expected_mutual_information(contingency, n_samples): | ||
""" Calculate the expected mutual information for two labellings. """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo (the U.S. spelling with a single "l" is more common) and PEP257 style:
"""Calculate the expected mutual information for two labelings."""
I have added new items to the TODO list. |
Conflicts: sklearn/metrics/cluster/__init__.py
- See Also sections updated - mutual_information -> mutual_information_score (and updating subsequent imports)
…sted_for_chance_measures.py example Commiting to get help on error
There is an overflow problem, which happens when running edit #2: I think I've fixed it. I need to test against the matlab code with larger arrays, but I have to do that on my other computer. |
This fixes the tests (which needed a bit of updating)
I'm not familiar with the style of plots used in the |
Apart from the issue with |
Sorry for the late reply, I will try to have a look at the plot_adjusted_for_chance_measures stuff tomorrow. |
Here are the pictures I get: The legends are good. However the AMI score should always be zero (as the ARI) which is not the case. This might be caused by the following warnings I get when computing the AMI scores:
Also it seems that the AMI scores are much slower to compute than ARI and V-Measure. Would be great to try and optimize the runtime once the incorrect value issues are fixed. |
Also here is the outcome of the doctests:
|
Don't check this yet - I didn't commit one lot from home and need to do that first (it updated the examples, which don't work now!) |
…to ami Conflicts: doc/whats_new.rst
Ready to be checked. I added a note in the docstring of |
@@ -19,7 +19,7 @@ Changelog | |||
- Faster tests by `Fabian Pedregosa`_. | |||
|
|||
- Silhouette Coefficient cluster analysis evaluation metric added as | |||
``sklearn.metrics.silhouette_score`` by Robert Layton. | |||
``sklearn.metrics.silhouette_score`` by `Robert Layton`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you to make your name a link, you need to add a trailing _
to it.
Looks good. +1 for merge. |
Merge when ready. |
I am doing changes on this pull request right now :) |
Adjusted Mutual Information: Mutual information adjusted for chance
I made a mistake sending my mail, and it didn't get to the pull request. --Earlier mail-- I was about to merge, after integrating the changes that I made on my nosetests -s --with-doctest --doctest-tests --doctest-extension=rst \ --doctest-fixtures=_fixture doc/ doc/modules/ DeprecationWarning) .F......F... ====================================================================== FAIL: Doctest: clustering.rst ---------------------------------------------------------------------- Traceback (most recent call last): File "/usr/lib/python2.6/doctest.py", line 2163, in runTest raise self.failureException(self.format_failure(new.getvalue())) AssertionError: Failed doctest test for clustering.rst File "/home/varoquau/dev/scikit-learn/doc/modules/clustering.rst", line 0 ---------------------------------------------------------------------- File "/home/varoquau/dev/scikit-learn/doc/modules/clustering.rst", line 493, in clustering.rst Failed example: metrics.adjusted_mutual_info_score(labels_true, labels_pred) # doctest: +ELLIPSIS Expected: 0.24... Got: 0.22504228319830874 ---------------------------------------------------------------------- File "/home/varoquau/dev/scikit-learn/doc/modules/clustering.rst", line 500, in clustering.rst Failed example: metrics.adjusted_mutual_info_score(labels_true, labels_pred) # doctest: +ELLIPSIS Expected: 0.24... Got: 0.22504228319830874 ---------------------------------------------------------------------- File "/home/varoquau/dev/scikit-learn/doc/modules/clustering.rst", line 507, in clustering.rst Failed example: metrics.adjusted_mutual_info_score(labels_pred, labels_true) # doctest: +ELLIPSIS Expected: 0.24... Got: 0.22504228319830874 ---------------------------------------------------------------------- File "/home/varoquau/dev/scikit-learn/doc/modules/clustering.rst", line 520, in clustering.rst Failed example: metrics.adjusted_mutual_info_score(labels_true, labels_pred) # doctest: +ELLIPSIS Expected: 0.0... Got: Got: -0.10526315789473642 >> raise self.failureException(self.format_failure(> instance at 0x9c86b8c>.getvalue())) |
Checking now (I re-read your comment - I thought you meant "merge when ready, I'm already working off a branch of this"! Sorry about that). |
Thanks. I've merged the changes that I had made in my fork of your branch |
Issue is confirmed, which is weird - it only happened after I merged the branches :S I think -- this may be a problem with some of my libraries. I had an issue previously where doctests weren't showing up, despite everything else working. I'll fix that problem independently. I have the matlab code for AMI, so I'll double check the doctests versus that code. |
The good news is that you can reproduce it. :$. Maybe you can do a |
After checking with the matlab code, these values are wrong, but the code is right! Checking the build now. Do I start a new PR? |
So, you are saying that the tests are wrong?
Don't start a new PR: this is bug fixing, and I don't think that it needs |
Yup, tests were wrong. I'll look back over the code to work out why I did that, but its more important just to get the fix in, so that the build is good (as you said). The fix works (just the values are changed, and I took them straight from matlab). I'll push to master in a second (I'll double check it won't break anything else!). |
Fair enough. Looks like we don't run the tests enough when reviewing pull |
If it gives you any comfort, I pretty much did the same thing in my pull |
That does give me comfort. Thanks for checking though -- broken tests are bad! @ogrisel It was my understanding that the AMI is bounded between 0 and 1. However one of the examples in doctests gives a negative score -- and it does with the matlab code as well! Any thoughts? |
Negative score can happen for "random-like" labelings because with are doing a diff with the expected value of random labelings. It is just never occurring in practice if your are evaluating a cluster algorithm that does its job of finding slightly above average clustering. |
Fair enough. That had me a little worried (but it was what the matlab code had...) |
Mutual Information, adjusted for chance. See [1] for details (specifically, look at the references for detail)
I have tested this against the Matlab code, and it works! Took me a while, as I had log for entropy and log2 for Expected Mutual Information (should be the other way around). I think that the
_expected_mutual_information
can be optimised, but I went with "get it right" first.[1] http://en.wikipedia.org/wiki/Adjusted_Mutual_Information