Skip to content

Try to fix experiment already exist issue in MLFlowHandler - #7916

Merged
KumoLiu merged 7 commits into
Project-MONAI:devfrom
KumoLiu:nvflare-mlflow
Jul 18, 2024
Merged

Try to fix experiment already exist issue in MLFlowHandler#7916
KumoLiu merged 7 commits into
Project-MONAI:devfrom
KumoLiu:nvflare-mlflow

Conversation

@KumoLiu

@KumoLiu KumoLiu commented Jul 14, 2024

Copy link
Copy Markdown
Contributor

Try to fixes NVIDIA/NVFlare#2698.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Comment thread monai/handlers/mlflow_handler.py
Comment thread monai/handlers/mlflow_handler.py
@binliunls

Copy link
Copy Markdown
Contributor

I think one more thing to discuss is whether MONAI should handle the multiprocessing scene of the mlflow handler or users should implement it in their code. If we'd like to do this, we'd better to use a more formal way like a lock for this.

Thanks
Bin

KumoLiu added 2 commits July 16, 2024 15:09
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu

KumoLiu commented Jul 16, 2024

Copy link
Copy Markdown
Contributor Author

I think one more thing to discuss is whether MONAI should handle the multiprocessing scene of the mlflow handler or users should implement it in their code. If we'd like to do this, we'd better to use a more formal way like a lock for this.

A good point. I didn't find a multiprocessing scene, the issue here is that in the nvflare real world tests, it will raise an error during creating experiments in different sites. Perhaps we can add try-catch block here and if there is a real-world case, we can refactor the logic. What do you think? Thanks.

@binliunls

Copy link
Copy Markdown
Contributor

I think one more thing to discuss is whether MONAI should handle the multiprocessing scene of the mlflow handler or users should implement it in their code. If we'd like to do this, we'd better to use a more formal way like a lock for this.

A good point. I didn't find a multiprocessing scene, the issue here is that in the nvflare real world tests, it will raise an error during creating experiments in different sites. Perhaps we can add try-catch block here and if there is a real-world case, we can refactor the logic. What do you think? Thanks.

Sure, we can leave this one as it is. But I think from the flare side, they need to handle this multiprocessing issue.

Comment thread monai/handlers/mlflow_handler.py Outdated
KumoLiu added 2 commits July 17, 2024 13:26
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu

KumoLiu commented Jul 17, 2024

Copy link
Copy Markdown
Contributor Author

/build

4 similar comments
@KumoLiu

KumoLiu commented Jul 17, 2024

Copy link
Copy Markdown
Contributor Author

/build

@KumoLiu

KumoLiu commented Jul 17, 2024

Copy link
Copy Markdown
Contributor Author

/build

@KumoLiu

KumoLiu commented Jul 17, 2024

Copy link
Copy Markdown
Contributor Author

/build

@Nic-Ma

Nic-Ma commented Jul 17, 2024

Copy link
Copy Markdown
Contributor

/build

@KumoLiu

KumoLiu commented Jul 18, 2024

Copy link
Copy Markdown
Contributor Author

/build

1 similar comment
@YanxuanLiu

Copy link
Copy Markdown
Collaborator

/build

@KumoLiu

KumoLiu commented Jul 18, 2024

Copy link
Copy Markdown
Contributor Author

/build

@KumoLiu
KumoLiu enabled auto-merge (squash) July 18, 2024 06:45
@KumoLiu
KumoLiu merged commit 85ab9f4 into Project-MONAI:dev Jul 18, 2024
@KumoLiu
KumoLiu deleted the nvflare-mlflow branch July 18, 2024 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] RESOURCE_ALREADY_EXISTS: Experiment 'monai_nvflare' already exists.

6 participants