-
-
Notifications
You must be signed in to change notification settings - Fork 650
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
Add job return logging #2111
Add job return logging #2111
Conversation
8948e68
to
33cbcb1
Compare
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.
Is this a callback we might be able to enable by default?
I.e. what are the disadvantages of enabling it?
Does it result in more console output?
In any case the code looks good. thanks
import {ExampleGithubLink} from "@site/src/components/GithubLink" | ||
|
||
<ExampleGithubLink text="Examples" to="hydra/experimental"/> |
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 think users might be confused when this "Examples" button redirects to the hydra/experimental
directory; there are some things in that directory that are unrelated to callbacks.
What if we create a file hydra/experimental/callbacks.py
that contains all the callback implementations (instead of using separate files log_job_return_callback.py
and pickle_job_info_callback.py
)? The ExampleGithubLink
in the docs could then point to that specific file callbacks.py
.
This would make the yaml files simpler too: users would write _target_: hydra.experimental.callbacks.LogJobReturnCallback
instead of _target_: hydra.experimental.log_job_return_callback.LogJobReturnCallback
.
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.
That's a great idea! Since this would mean touching the PickleJobCallback as well, I will create that PR as a follow up.
For now I will remove this change.
It does result in more console output. Here is an example:
|
I tried a fews ways to add the logging in hydra core without a callback. However didn't find a clean way to add the logging in both RUN and MULTIRUN mode without adding much more noise to the console.
Add logging as a callback means: 1) job return will be logged together with the rest of the application log, 2) users can opt in.
related to #2100