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.

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

Decouple evaluation metrics from tuning objectives #384

Merged
merged 23 commits into from
Nov 9, 2017

Conversation

desilinguist
Copy link
Member

@desilinguist desilinguist commented Nov 7, 2017

Right now, the only metrics computed for any SKLL experiment are the tuning objectives. However, one may want to compute additional evaluation metrics without tuning on them. This PR makes that possible.

  • For evaluate and cross_validate tasks, one can now specify a metrics list in the Output section and those will be computed and saved at the end of the results file under an "Additional Evaluation Metrics" section. For train and predict tasks, metrics is ignored since it's not relevant.

  • For learning_curve tasks, it doesn't actually make sense to have objectives anyway so you can specify metrics instead. Using objectives is still supported for backward compatibility but you will get a deprecation warning. (Note: as an implementation detail, we still use the "objectives" variable internally because then we can piggyback on that to parallelize the various jobs.)

  • The parsing and validation of objectives and metrics is now factored out into a separate function to avoid code duplication.

  • Update documentation.

  • Add several new tests and update existing tests to deal with the new additional metrics being produced in the outputs.

- Factor out metric parsing and validation into a seprate function that is now used for both the "objectives" option and this new "metrics" option.
- Add several checks to raise warnings and errors depending on the task and whether both "objectives" and "metrics" are specified.
- Pass in "metrics" to `run_configuration()`.
- Used in the `evaluate()` and `cross_validate()` tasks.
- Return the additional values in a dictionary that's part of the task results.
- Change the argument name in `learning_curve()` to be metric instead of `objective` since that makes more sense (there's no tuning).
- Handle receiving the metrics and passing them to the appropriate Learner methods.
- Deal with the additional output dictionary when creating results files.
- For the learning curve task, even though objectives are not actually used a objectives, we piggyback on that set up since then we can parallelize all of the metrics.
- Since we need the metrics list to be populated for later.
- Also update the actual CFG file from the example
@coveralls
Copy link

coveralls commented Nov 7, 2017

Coverage Status

Coverage increased (+0.3%) to 92.334% when pulling c36a519 on decouple-metrics-from-objectives into 28a80b1 on master.

skll/config.py Outdated
logger.warning("The \"objectives\" option "
"is deprecated for the learning_curve "
"task and will not be supported "
"starting with the next release; please "
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe "after" instead of "starting with"?

skll/config.py Outdated
elif task in ['evaluate', 'cross_validate']:
# for other appropriate tasks, if metrics and objectives have
# some overlaps - we will assume that the user meant to include
# use the metric for tuning _and_ evaluation, not just evaluation
Copy link
Collaborator

Choose a reason for hiding this comment

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

use -->

@coveralls
Copy link

coveralls commented Nov 8, 2017

Coverage Status

Coverage increased (+0.3%) to 92.334% when pulling 86031ab on decouple-metrics-from-objectives into abf1db5 on master.

@desilinguist
Copy link
Member Author

desilinguist commented Nov 8, 2017

@aoifecahill @mulhod @cml54 @benbuleong @Lguyogiro this PR is now ready for review.

@desilinguist desilinguist merged commit dd28d00 into master Nov 9, 2017
@desilinguist desilinguist deleted the decouple-metrics-from-objectives branch November 9, 2017 21:45
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.

None yet

5 participants