Allow specifying custom seed for cross-validation #707
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Add a new `cv_seed` keyword argument to `Learner.cross_validate()` method with the same default value as before.
- This field should be in the "Input" section. - Add it to the documentation. - Show the seed in the experiment log.
- Add a new test generator that sets a custom seed and then checks whether the folds generated from inside SKLL match expected folds, for both classifiers and regressors. - This test requires computing the expected folds which we were already doing for another, already existing test. So, I factored that code out into a utility function in `tests/utils.py`.
- Also add tests for the voting learners side in the same vein as the regular learners using the same utility function.
- `test_voting_learners_api_5.py` now takes 2x longer due to the addition of a new boolean field so let's move it to a different test partition which is much shorter than others.
desilinguist requested review from a user, Frost45, damien2012eng and mulhod 
Codecov Report
@@ Coverage Diff @@ ## main #707 +/- ## ========================================== + Coverage 96.89% 96.92% +0.03% ========================================== Files 63 63 Lines 9199 9263 +64 ========================================== + Hits 8913 8978 +65 + Misses 286 285 -1
Continue to review full report at Codecov.
|
Frost45 approved these changes Dec 17, 2021
Contributor
Frost45 left a comment
Frost45 left a comment There was a problem hiding this comment.
Looks great @desilinguist! A few minor suggestions.
Collaborator Author
| @Frost45 please take a look at the changes! |
Frost45 approved these changes Dec 17, 2021
Contributor
Frost45 left a comment
Frost45 left a comment There was a problem hiding this comment.
Looks great @desilinguist! Thank you for doing this! 🙏🏼
Co-authored-by: Sanjna Kashyap <20379363+Frost45@users.noreply.github.com>
mulhod approved these changes Dec 20, 2021
delete-merged-branch bot deleted the 593-allow-specifying-seed-for-xval branch Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
This PR closes #593.
cv_seedkeyword argument to theLearner.cross_validate()method with the same default value as before.cv_seedconfiguration file field in the “Input” section. Also, show the seed in the experiment log.tests/utils.py.cv_seedfor voting learners too.test_voting_learners_api_5.pynow takes 2x longer due to the addition of a new boolean field so let's move it to a different test partition which is much shorter than others.