Skip to content

Use 5 folds for grid search - #667

Merged
desilinguist merged 5 commits into
mainfrom
use-5-folds-for-grid-search
Feb 23, 2021
Merged

Use 5 folds for grid search#667
desilinguist merged 5 commits into
mainfrom
use-5-folds-for-grid-search

Conversation

@desilinguist

@desilinguist desilinguist commented Feb 22, 2021

Copy link
Copy Markdown
Collaborator

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

codecov Bot commented Feb 22, 2021

Copy link
Copy Markdown

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
Copy Markdown
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

@aoifecahill aoifecahill left a comment

Copy link
Copy Markdown
Collaborator

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

3 participants