-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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] Custom Logging, Trial Name #3465
Conversation
Test FAILed. |
custom_loggers (list): List of custom logger creators. | ||
sync_cmd_tmpl (str): Optional template for syncer to run. Needs to | ||
include replacement fields "{local_dir}" and "{remote_dir}". | ||
See ray/python/ray/tune/log_sync.py |
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.
Nice! I've been thinking that it would also be nice to have something similar for the trial logs folder names. Currently, many of my grid_search
values include tuples etc., which tend to clutter the log dirs, while I usually only care about the id of the trial.
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.
Hey @hartikainen, can you leave some feedback in #3034? The sentiment there is similar, but it would be great to get some consolidated feedback before implementing.
Test PASSed. |
Test FAILed. |
Test FAILed. |
cc @llan-ml, @hartikainen This addresses issues and requests you've made. It would be really great if you could take a glance at this PR! |
Test FAILed. |
|
||
|
||
def trial_str_creator(trainable_name, trial_id, config): | ||
return "{}_{}_123".format(trainable_name, trial_id) |
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.
How about we just pass the trial object here?
python/ray/tune/config_parser.py
Outdated
@@ -106,6 +106,21 @@ def make_parser(parser_creator=None, **kwargs): | |||
default="", | |||
type=str, | |||
help="Optional URI to sync training results to (e.g. s3://bucket).") | |||
parser.add_argument( | |||
"--trial-string-creator", |
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.
trial_name_creator?
python/ray/tune/config_parser.py
Outdated
parser.add_argument( | ||
"--custom-loggers", | ||
default=None, | ||
help="Optional URI to sync training results to (e.g. s3://bucket).") |
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.
Help is wrong.
default="", | ||
type=str, | ||
help="Optional template for syncer to run. Needs to " | ||
"include replacement fields '{local_dir}' and '{remote_dir}'.") |
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.
Instead of a string how about just have sync_function
here. You can also say "btw if you pass a string it will be used by this default function".
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.
done
Test FAILed. |
Test FAILed. |
Test FAILed. |
Apart from @ericl's comments, this looks good to me. I really like it, thanks @richardliaw! |
LGTM! |
Test FAILed. |
|
||
Returns: | ||
trial_name (str): String representation of Trial. | ||
""" |
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.
Missing body.
doc/source/tune-usage.rst
Outdated
.. code-block:: python | ||
|
||
def custom_sync_func(local_dir, remote_dir): | ||
# run sync |
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.
Example
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
jenkins retest this please |
Test FAILed. |
jenkins retest this please |
Test FAILed. |
Test PASSed. |
TODO:
This should address #3034, #2985, and #3390.