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

Confidence Intervals for Permutation Feature Importance #1844

Merged
merged 10 commits into from
Dec 20, 2018

Conversation

rogancarr
Copy link
Contributor

@rogancarr rogancarr commented Dec 6, 2018

This PR adds an optional confidence interval calculation to the Permutation Feature Importance evaluator. If the users specifies the number of permutations > 1, then the resulting evaluating metrics will contain the mean and standard deviation.

Fixes #1840

Update: In this PR, I am removing PFI for clustering. It actually doesn't make sense to use clustering metrics with PFI because cluster membership can change during permutations. If we want to look at feature importance for clustering, it'll be better to use a test specific to clustering.

@rogancarr
Copy link
Contributor Author

@glebuk @TomFinley I'd appreciate a look at this if possible. I added a new class that returns descriptive statistics. What do you two think?

…tics package. Updated the sample to use the standard error.
@rogancarr rogancarr changed the title WIP: Confidence Intervals for Permutation Feature Importance Confidence Intervals for Permutation Feature Importance Dec 7, 2018
public abstract class MetricsStatisticsBase<T>{
internal MetricsStatisticsBase()
{
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why you need this constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to make this class public, but I don't want it to be inheritable outside this namespace. Is there a better way to restrict it?

Copy link
Contributor

@glebuk glebuk left a comment

Choose a reason for hiding this comment

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

:shipit:

@rogancarr rogancarr merged commit 41d3196 into dotnet:master Dec 20, 2018
@rogancarr rogancarr deleted the 1840_PfiConfidenceIntervals branch December 20, 2018 02:02
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants