-
Notifications
You must be signed in to change notification settings - Fork 19
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
Provide internal implementation of OptunaSearchCV #265
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #265 +/- ##
=======================================
Coverage 90.13% 90.13%
=======================================
Files 54 54
Lines 2423 2423
Branches 491 491
=======================================
Hits 2184 2184
Misses 157 157
Partials 82 82
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
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.
Looks good to me, and it might be worth to exempt this from formatting, linting, testing and code coverage.
Since the current
OptunaSearchCV
has an issue with respect of scikit-learn's implementation (optuna/optuna-integration#118), this PR includes an internal implementation which is fully compatible with scikit-learn.Particularly, the
fit
method uses a different study in each call, thus respecting the scikit-learn convention of learning in each call and overriding any previous fit call. The implementation in the optuna-integration package will continue training. This creates a test-to-train leakage when used with scikit-learn'scross_validate
function, which is also used by julearn.