Skip to content

Conversation

@desilinguist
Copy link
Collaborator

@desilinguist desilinguist commented Feb 22, 2021

This PR closes #487.

See issue for motivation to change the default number of folds for grid search cross-validation. One by-product of the change is that experiments will certainly take longer to run but they are likely to be more reliable.

Changes:

  • Change default grid search folds from 3 to 5.
  • Update grid_search_folds documentation
  • Change default value of MAX_SKLL_CONCURRENT_PROCESSES to 5.
  • Change tests to use 3 grid search folds otherwise tests take too long. Note that the the config parsing tests in test_input.py were correctly modified to check that the default value for grid search folds is 5.

- Update `grid_search_folds` documentation
- Also change default value of `MAX_SKLL_CONCURRENT_PROCESSES` to 5.
Otherwise tests take too long. Note that the the config parsing tests in `test_input.py` were correctly modified to check that the default value for grid search folds is 5.
@codecov
Copy link

codecov bot commented Feb 22, 2021

Codecov Report

Merging #667 (ea4966d) into main (dc5ed05) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #667   +/-   ##
=======================================
  Coverage   96.76%   96.76%           
=======================================
  Files          63       63           
  Lines        9077     9077           
=======================================
  Hits         8783     8783           
  Misses        294      294           
Impacted Files Coverage Δ
skll/config/__init__.py 95.11% <ø> (ø)
skll/learner/__init__.py 97.09% <ø> (ø)
skll/learner/voting.py 98.42% <ø> (ø)
tests/test_input.py 99.79% <ø> (ø)
tests/test_output.py 100.00% <ø> (ø)
tests/test_voting_learners_api_1.py 100.00% <ø> (ø)
tests/test_voting_learners_api_2.py 100.00% <ø> (ø)
tests/test_voting_learners_api_3.py 100.00% <ø> (ø)
tests/test_voting_learners_api_5.py 100.00% <ø> (ø)
tests/test_voting_learners_expts_1.py 98.38% <ø> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc5ed05...ea4966d. Read the comment docs.

@delete-merged-branch delete-merged-branch bot deleted the branch main February 23, 2021 01:58
@desilinguist desilinguist changed the base branch from add-voting-learners to main February 23, 2021 02:00
@desilinguist
Copy link
Collaborator Author

@mulhod it'd be really great if you could test this with gridmap as well. Thanks!

@desilinguist desilinguist marked this pull request as ready for review February 23, 2021 14:10
@desilinguist desilinguist requested review from a user, aoifecahill and mulhod February 23, 2021 14:10
Copy link
Collaborator

@aoifecahill aoifecahill left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@desilinguist desilinguist merged commit 86781f1 into main Feb 23, 2021
@delete-merged-branch delete-merged-branch bot deleted the use-5-folds-for-grid-search branch February 23, 2021 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change default grid search folds

4 participants