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.

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

Remove experimental decorators from hyperparameter importance (HPI) features. #1440

Merged
merged 2 commits into from Jun 30, 2020
Merged

Remove experimental decorators from hyperparameter importance (HPI) features. #1440

merged 2 commits into from Jun 30, 2020

Conversation

hvy
Copy link
Member

@hvy hvy commented Jun 26, 2020

Motivation

Hyperparameter importance (HPI) APIs should be made stable before v2.0.

Description of the changes

  • Aligns somewhat inconsistent APIs
  • Removes experimental decorators
  • Adds tests

@hvy hvy added the feature Change that does not break compatibility, but affects the public interfaces. label Jun 26, 2020
@hvy hvy added this to the v2.0.0 milestone Jun 26, 2020
@hvy hvy mentioned this pull request Jun 29, 2020
Closed
@toshihikoyanase toshihikoyanase self-assigned this Jun 29, 2020
@HideakiImamura HideakiImamura self-assigned this Jun 30, 2020
toshihikoyanase
toshihikoyanase requested changes Jun 30, 2020
View reviewed changes
Copy link
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

Thank you for your PR. It looks good to me except for small comments.

tests/importance_tests/test_mean_decrease_impurity.py Outdated Show resolved Hide resolved
tests/importance_tests/test_fanova.py Outdated Show resolved Hide resolved
@hvy @toshihikoyanase
Apply suggestions from code review
d558299 
Co-authored-by: Toshihiko Yanase <toshihiko.yanase@gmail.com>
@hvy
Copy link
Member Author

hvy commented Jun 30, 2020 •
edited

Thanks for catching my careless mistakes. I applied your suggestions, PTAL.

HideakiImamura
HideakiImamura approved these changes Jun 30, 2020
View reviewed changes
Copy link
Member

@HideakiImamura HideakiImamura left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The changes are LGTM.

toshihikoyanase
toshihikoyanase approved these changes Jun 30, 2020
View reviewed changes
Copy link
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

Thank you for your update. LGTM.

@toshihikoyanase toshihikoyanase merged commit 6a16746 into optuna:master Jun 30, 2020
@hvy hvy deleted the hpi-rm-experimental branch June 30, 2020 14:57
@hvy hvy mentioned this pull request Jul 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toshihikoyanase toshihikoyanase toshihikoyanase approved these changes

@HideakiImamura HideakiImamura HideakiImamura approved these changes

Assignees

@toshihikoyanase toshihikoyanase

@HideakiImamura HideakiImamura

Labels
feature Change that does not break compatibility, but affects the public interfaces.
Projects
None yet
Milestone
v2.0.0-rc0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hvy @toshihikoyanase @HideakiImamura