-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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] logger migration to ExperimentLogger classes #11984
[tune] logger migration to ExperimentLogger classes #11984
Conversation
…tion # Conflicts: # python/ray/tune/examples/wandb_example.py
@@ -204,7 +204,7 @@ def on_result(self, result): | |||
} | |||
} | |||
}) | |||
self.assertTrue( | |||
self.assertFalse( |
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.
I thought about this and I think it makes sense to never add any logger callbacks if someone sets the TUNE_DISABLE_AUTO_CALLBACK_LOGGERS
env variable to 1. That's a deliberate choice and we shouldn't mess with that.
Before this PR there was a reason to add this (since we distinguished between legacy Logger
classes and callbacks), but since we're moving to all-callbacks now, there's no need to do magic here.
@@ -249,7 +246,9 @@ def run( | |||
trial (of ERROR state) when the experiments complete. | |||
callbacks (list): List of callbacks that will be called at different | |||
times in the training loop. Must be instances of the | |||
``ray.tune.trial_runner.Callback`` class. | |||
``ray.tune.trial_runner.Callback`` class. If not 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.
``ray.tune.trial_runner.Callback`` class. If not passed, | |
``ray.tune.callback.Callback`` class. If not 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.
this looks great - i'll merge and we can fix the docstring in a later PR
Why are these changes needed?
Moves from
Logger
classes toExperimentLogger
classes.Todo:
tune.loggers
Related issue number
Checks
scripts/format.sh
to lint the changes in this PR.