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

[tune] use default anonymous metric _metric if at least a mode is set #12159

Merged
merged 8 commits into from
Nov 24, 2020

Conversation

krfricke
Copy link
Contributor

Why are these changes needed?

Ray Tune already supports an anonymous metric if e.g. tune.report(0.5) is called or a scalar value is returned in the (function) trainable. However, it is currently not used automatically, i.e. users will have to specify tune.run(train, mode="max", metric="_metric") to use this metric in the searchers, schedulers, progress reporting, and experiment analysis.

This PR changes this behavior so that the default _metric is used when no other metric has been specified but a mode has been set. Thus, the API reduces to tune.run(train, mode="max").

I think it's sensible to require setting a mode, as we cannot assume if we're optimizing for higher or lower values. Other libraries make this assumption, or default to one direction. Also, by explicitly setting a mode but not a metric, we can assume that the user actually wants to optimize an anonymous metric.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@krfricke krfricke added the tune Tune-related issues label Nov 19, 2020
# Conflicts:
#	python/ray/tune/tests/test_searchers.py
Copy link
Contributor

@richardliaw richardliaw left a comment

Choose a reason for hiding this comment

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

looks like a kosher change though tests are failing?

Mainly comments regarding docstring.

python/ray/tune/analysis/experiment_analysis.py Outdated Show resolved Hide resolved
python/ray/tune/schedulers/median_stopping_rule.py Outdated Show resolved Hide resolved
python/ray/tune/schedulers/pbt.py Outdated Show resolved Hide resolved
python/ray/tune/suggest/ax.py Show resolved Hide resolved
python/ray/tune/suggest/hyperopt.py Outdated Show resolved Hide resolved
@krfricke
Copy link
Contributor Author

Failing test is test_trial_scheduler_pbt and the error seems unrelated to this PR (occurs in other PRs, too)

@richardliaw richardliaw merged commit b94bfdf into ray-project:master Nov 24, 2020
@krfricke krfricke deleted the tune-default-metric branch November 24, 2020 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tune Tune-related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants