setup_mlflow API Ray Train V2 Compatibility#58705
setup_mlflow API Ray Train V2 Compatibility#58705justinvyu merged 15 commits intoray-project:masterfrom
Conversation
…p_mlflow Signed-off-by: JasonLi1909 <[email protected]>
There was a problem hiding this comment.
Code Review
This pull request aims to add compatibility with Ray Train V2 for the setup_mlflow API by handling deprecation warnings. The changes introduce try-except blocks to catch DeprecationWarning and fall back to alternative APIs or gracefully handle the situation.
While the intent is good, I've found a critical issue where the new structure could lead to a NameError if ray.train.get_context() raises a RuntimeError. I've provided a suggestion to restructure the code to prevent this.
Additionally, I've pointed out that simply catching DeprecationWarning for context methods like get_world_rank() and then proceeding without trial information could lead to silent failures in MLflow logging. It would be better to use the newer APIs if possible.
Signed-off-by: JasonLi1909 <[email protected]>
Signed-off-by: JasonLi1909 <[email protected]>
Signed-off-by: JasonLi1909 <[email protected]>
Signed-off-by: JasonLi1909 <[email protected]>
Signed-off-by: JasonLi1909 <[email protected]>
Signed-off-by: JasonLi1909 <[email protected]>
justinvyu
left a comment
There was a problem hiding this comment.
Thanks, just a few minor things
Signed-off-by: JasonLi1909 <[email protected]>
Signed-off-by: JasonLi1909 <[email protected]>
Signed-off-by: JasonLi1909 <[email protected]>
Signed-off-by: JasonLi1909 <[email protected]>
Co-authored-by: Justin Yu <[email protected]> Signed-off-by: Jason Li <[email protected]>
- Makes setup_mlflow compatible with Ray Train V2 by avoiding deprecated API usage - Adds tests for testing the compatibility of Train V2 with setup_mlflow --------- Signed-off-by: JasonLi1909 <[email protected]> Signed-off-by: Jason Li <[email protected]> Co-authored-by: Justin Yu <[email protected]>
- Makes setup_mlflow compatible with Ray Train V2 by avoiding deprecated API usage - Adds tests for testing the compatibility of Train V2 with setup_mlflow --------- Signed-off-by: JasonLi1909 <[email protected]> Signed-off-by: Jason Li <[email protected]> Co-authored-by: Justin Yu <[email protected]> Signed-off-by: YK <[email protected]>
- Makes setup_mlflow compatible with Ray Train V2 by avoiding deprecated API usage - Adds tests for testing the compatibility of Train V2 with setup_mlflow --------- Signed-off-by: JasonLi1909 <[email protected]> Signed-off-by: Jason Li <[email protected]> Co-authored-by: Justin Yu <[email protected]>
- Makes setup_mlflow compatible with Ray Train V2 by avoiding deprecated API usage - Adds tests for testing the compatibility of Train V2 with setup_mlflow --------- Signed-off-by: JasonLi1909 <[email protected]> Signed-off-by: Jason Li <[email protected]> Co-authored-by: Justin Yu <[email protected]> Signed-off-by: peterxcli <[email protected]>
Users have run into issues using the
setup_mlflowAPI with Ray Train V2 enabled:setup_mlflowin a tune run, aDeprecationWarningis raised becauseray.train.get_context()is called from with a Ray Tune runsetup_mlflowin a regular Train run, aDeprecationWarningis raised becauseTrainContext.get_trial_id()is called but this is no longer supported in Train V2This PR: