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] Checkpoint according to nested metric #14379

Merged
merged 1 commit into from Mar 1, 2021
Merged

[tune] Checkpoint according to nested metric #14379

merged 1 commit into from Mar 1, 2021

Conversation

ghost
Copy link

@ghost ghost commented Feb 26, 2021

Why are these changes needed?

In tune, checkpoint according to a metric nested in the reported result.

This is useful when optimize a metric of evaluation in rllib.

My solution is similar to #14375

Related issue number

Closes #14377

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 :(

@ghost
Copy link
Author

ghost commented Feb 27, 2021

I noticed that the unit test failed, but I don't quite understand error message.
At the end of log, it points out that 79 test pass and 1 test failed. But there are multiple lines indicating test failure.
Can anybody help me locating the error? I can try to solve it if I know exactly which test failed.

@krfricke
Copy link
Contributor

krfricke commented Mar 1, 2021

The Travis CI test is currently the one we use for tracking - sometimes other tests are flaky and can lead to failing CI. The changes from this PR apparently did not introduce any failures, so we should be good to merge

@krfricke krfricke merged commit 343ebf8 into ray-project:master Mar 1, 2021
@ghost
Copy link
Author

ghost commented Mar 2, 2021

Ah, got it. Thanks a lot!

@ghost ghost deleted the checkpoint_nested_metric branch June 26, 2021 15:20
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.

[tune] Checkpoint according to nested metric
1 participant