-
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
[Doc] Clarified when to use Mlflow logger and setup_mlflow; fix #36257 #37854
Conversation
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.
|
ping @krfricke I am not sure whether the tests are related to documentation |
Hi again! The issue will be closed because there has been no more activity in the 14 days since the last message. Please feel free to reopen or open a new issue if you'd still like it to be addressed. Again, you can always ask for help on our discussion forum or Ray's public slack channel. Thanks again for opening the issue! |
@krfricke Hi, just a quick ping, if I shall reopen the PR ? Totally understand, that this is not an important issue |
@matthewdeng can you follow up. |
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.
Thanks 😄
@@ -45,8 +45,8 @@ def setup_mlflow( | |||
) -> Union[ModuleType, _NoopModule]: | |||
"""Set up a MLflow session. | |||
|
|||
This function can be used to initialize an MLflow session in a | |||
(distributed) training or tuning run. | |||
This function can be used to initialize a MLflow session in a |
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: This should still be an
since the M starts with a vowel sound.
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 in the last commit
Keep in mind, that the callback will open a Mlflow session on the driver and | ||
not on the trainable. Therefore, it is not possible to call mlflow methods | ||
like ``mlflow.log_figure()`` inside the trainable as there is no Mlflow session | ||
on the trainable. Use :func:`setup_mlflow` if you need more fine grained control. | ||
|
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:
Keep in mind that the callback will open an MLflow session on the driver and
not on the trainable. Therefore, it is not possible to call MLflow functions
like ``mlflow.log_figure()`` inside the trainable as there is no MLflow session
on the trainable. For more fine grained control, use :func:`setup_mlflow`.
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 in the last commit
…roject#36257 Signed-off-by: Hannes Hansen <mail@hannesh.de>
Hi @matthewdeng Sorry for the late response! I applied your changes, thank you! |
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.
Thanks for the contribution! It looks like the docs are not rendering properly due to a different reason, will address that separately.
…roject#36257 (ray-project#37854) Signed-off-by: Hannes Hansen <mail@hannesh.de>
Why are these changes needed?
Information was missing about when to use
Mlflow Callback
orsetup_mlflow
. I added the explanation of @krfricke.Related issue number
See #36257
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.