Skip to content

[MRG] fix for NMI and AMI are not explained well in the user guide #8645 #8912

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

[MRG] fix for NMI and AMI are not explained well in the user guide #8645 #8912

wants to merge 5 commits into from

Conversation

shivamgargsya
Copy link
Contributor

@shivamgargsya shivamgargsya commented May 22, 2017 •
edited by lesteve
Loading

fix #8645
Added documentation for explanation for of normalized mutual information and adjusted mutual information.
Added examples illustrating the difference between these three parameters (mutual information, normalized mutual information and Adjusted mutual information)

jnothman
jnothman reviewed May 22, 2017
View reviewed changes
doc/modules/clustering.rst Outdated
@@ -1070,6 +1069,33 @@ Bad (e.g. independent labelings) have non-positive scores::
>>> metrics.adjusted_mutual_info_score(labels_true, labels_pred) # doctest: +ELLIPSIS
-0.10526...

Mutual Information has a problem that it does not penalizes large cardinalities.It
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please be sure to include whitespace between sentences

@shivamgargsya shivamgargsya changed the title fix for NMI and AMI are not explained well in the user guide #8645 [MRG] fix for NMI and AMI are not explained well in the user guide #8645 May 22, 2017
@lesteve
Copy link
Member

lesteve commented May 23, 2017

FYI I have edited your PR description to have "Fix #issueNumber". The regex is a bit finicky so fix: #issueNumber (additional colon) or fix issue #issueNumber is not going to work. For more details, look at this.

@lesteve
Copy link
Member

lesteve commented May 23, 2017 •
edited
Loading

The Travis errors seem genuine. You need to use metrics.normalized_mutual_info_score rather than normalized_mutual_info_score and metrics.adjusted_mutual_info_score and adjusted_mutual_info_score in the doctests.

@shivamgargsya
Copy link
Contributor Author

Hii @lesteve thanks for comments I have resolved them in new iteration.Can you point me to any documentation regarding circleci tests like some checkpoints to take care while raising a PR so that it passes circleci tests.

@amueller
Copy link
Member

This is fixed in #11124.

@amueller amueller closed this Sep 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnothman jnothman jnothman left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NMI and AMI are not explained well in the user guide
4 participants
@shivamgargsya @lesteve @amueller @jnothman