-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[rllib] on_train_result results do not get logged #3865
Conversation
Test FAILed. |
jenkins retest this please |
Test PASSed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need to change L426 in Trial.py for when you're running with run_experiments
right?
What do you mean? |
The logging in Tune happens in Trial (on driver) even when distributed (and
remote workers have a NoopLogger), so do you need to make modifications
there?
Basically this codepath I dont think gets hit in Tune when using
run_experiments
…On Fri, Feb 1, 2019 at 3:31 PM Eric Liang ***@***.***> wrote:
What do you mean?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#3865 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEUc5aqrn87RzVa4FUS659kA4H_2-79cks5vJM5TgaJpZM4aT4no>
.
|
That should work fine then. The only issue is if the result is logged
*before* the callback gets called. If Tune is getting it over remote call,
it the callback must have fired already.
On Fri, Feb 1, 2019 at 3:51 PM Richard Liaw <notifications@github.com>
wrote:
… The logging in Tune happens in Trial (on driver) even when distributed (and
remote workers have a NoopLogger), so do you need to make modifications
there?
Basically this codepath I dont think gets hit in Tune when using
run_experiments
On Fri, Feb 1, 2019 at 3:31 PM Eric Liang ***@***.***>
wrote:
> What do you mean?
>
> —
> You are receiving this because you were assigned.
>
>
> Reply to this email directly, view it on GitHub
> <#3865 (comment)>,
or mute
> the thread
> <
https://github.com/notifications/unsubscribe-auth/AEUc5aqrn87RzVa4FUS659kA4H_2-79cks5vJM5TgaJpZM4aT4no
>
> .
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3865 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAA6SoOPQilc74JEPtAhQl8SiN5TOV5Fks5vJNMQgaJpZM4aT4no>
.
|
What do these changes do?
Move the callback to before calling logger.on_result(). This requires a bit of a refactoring.
Related issue number
Closes #3854