-
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
[Train] Add PrintCallback
#21261
[Train] Add PrintCallback
#21261
Conversation
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.
LGTM @amogkam can you merge?
class PrintCallback(TrainingCallback): | ||
"""A callback that prints training results to STDOUT. | ||
|
||
Example: |
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.
Can we change the example to show usage with Trainer
? Users shouldn't be calling handle_result
directly, so I think this docstring could be confusing.
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.
Good point, we can also take the handle_result
example and move it to the handle_result
docstring?
python/ray/train/callbacks/print.py
Outdated
>>> from ray import train | ||
>>> from ray.train import Trainer | ||
>>> from ray.train.callbacks import PrintCallback | ||
|
||
>>> def train_func(): | ||
... for i in range(2): | ||
... train.report(worker_idx=train.world_rank(), epoch=i) |
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.
nit: Preference on >>>
vs. ..code-block
formatting? Personally prefer to just be able to copy/paste the example and run it.
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.
My reasoning was that the >>>
formatting would be easier to show what's actually being printed out. If we do ..code-block
then we would have to show the stdout as a comment. But good point on being able to copy paste. I don't really have a strong preference here, so am happy to change it to code block.
Why are these changes needed?
Related issue number
Closes #20806
Checks
scripts/format.sh
to lint the changes in this PR.