Skip to content
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.

Sign up for GitHub

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

Jump to bottom

5821 6303 Optimize MonaiAlgo FL based on BundleWorkflow #6158

Merged
merged 43 commits into from
Apr 19, 2023
Merged

5821 6303 Optimize MonaiAlgo FL based on BundleWorkflow #6158

merged 43 commits into from
Apr 19, 2023

Conversation

Nic-Ma
Copy link
Contributor

@Nic-Ma Nic-Ma commented Mar 16, 2023 •
edited
Loading

part of #5821
Fixes #6303

Description

This PR simplified the MONAI FL MonaiAlgo module to leverage BundleWorkflow.
The main point is to decouple the bundle read / write related logic with FL module and use predefined required-properties.

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.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 16, 2023 •
edited
Loading

CC @holgerroth to the loop.
I think the MONAI FL module is still in "experimental" stage, so I made some slightly breaking change directly.
I already updated MonaiAlgoStats and its unit tests.

Thanks.

@Nic-Ma Nic-Ma changed the title [WIP] 5821 Optimize MonaiAlgo FL based on BundleWorkflow 5821 Optimize MonaiAlgo FL based on BundleWorkflow Mar 17, 2023
@Nic-Ma Nic-Ma marked this pull request as ready for review March 17, 2023 09:30
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 17, 2023

/black

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 17, 2023

/build

@Nic-Ma
[DLMED] update distributed tests
cba1490 
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 17, 2023

/black

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 17, 2023

/build

@Nic-Ma Nic-Ma requested review from wyli, holgerroth and ericspod and removed request for holgerroth March 17, 2023 10:40
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 17, 2023

Hi @holgerroth ,

I updated the MonaiAlgo and MonaiAlgoStats in this PR, mainly note:

  1. It decoupled bundle config processing from MonaiAlgo.
  2. Bundle don't need to write in JSON or YAML config anymore, any SupervisedTrainer based BundleWorkflow can work in the MonaiAlgo.
  3. I expect to set BundleWorkflow as a NVFlare component in the fl_client config file, and pass it to MonaiAlgo as an object arg. @KumoLiu confirmed it's doable.
  4. I dropped 2 minor features: (1) disable checkpoint loader, I feel this feature is very hacky, better to let users manually disable it in the bundle config directly before running FL program. (2) no trainer and no evaluator but get_weights, I think get_weights should work at least with trainer exists. Please feel free to correct me if I misunderstand.

Could you please help review it?

Thanks in advance.

@Nic-Ma Nic-Ma requested a review from KumoLiu March 17, 2023 10:51
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 20, 2023

/build

@wyli
Copy link
Contributor

wyli commented Mar 20, 2023

I think this pr will also close #4942?

holgerroth and others added 7 commits April 6, 2023 19:55
@holgerroth
add weight diff check
7de4ed7
@Nic-Ma
Merge pull request #462 from holgerroth/5821-optimize-monaialgo_diff_…
7a46063 
…check

add weight diff check
@Nic-Ma
Merge branch 'dev' into 5821-optimize-monaialgo
1cc92e2
@Nic-Ma
[DLMED] add checkpint disable
fab3313 
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
[DLMED] add disable checkpoint loader
f131f04 
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
[DLMED] fix test logging
28f19c5 
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
[DLMED] fix mypy
66f090e 
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Apr 7, 2023

/black

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Apr 7, 2023

/build

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Apr 9, 2023

/black

monai-bot and others added 3 commits April 9, 2023 14:22
@monai-bot
[MONAI] code formatting
d068e03 
Signed-off-by: monai-bot <monai.miccai2019@gmail.com>
@Nic-Ma
Merge branch 'dev' into 5821-optimize-monaialgo
0945aab
@Nic-Ma
[DLMED] add eval to dist test
f626f83 
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma Nic-Ma mentioned this pull request Apr 18, 2023
Closed
7 tasks
@Nic-Ma
[DLMED] optimize tests
19a10d8 
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Apr 18, 2023

/black

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Apr 18, 2023

/build

@Nic-Ma Nic-Ma requested a review from holgerroth April 18, 2023 07:56
holgerroth
holgerroth approved these changes Apr 18, 2023
View reviewed changes
Copy link
Collaborator

@holgerroth holgerroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Tested with nvflare 2.3.0 and works fine.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Apr 18, 2023

/build

@Nic-Ma Nic-Ma enabled auto-merge (squash) April 18, 2023 23:54
@Nic-Ma Nic-Ma merged commit d8eb68a into Project-MONAI:dev Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@holgerroth holgerroth holgerroth approved these changes

@wyli wyli Awaiting requested review from wyli

@ericspod ericspod Awaiting requested review from ericspod

@KumoLiu KumoLiu Awaiting requested review from KumoLiu

Assignees

@Nic-Ma Nic-Ma

Labels
None yet
Projects
No open projects
MONAI v1.2
Status: Done
Milestone
Bundle class [P1 v1.2]
Development

Successfully merging this pull request may close these issues.

MonaiAlgo's convert_global_weights broken for multi-gpu clients
4 participants
@Nic-Ma @wyli @holgerroth @monai-bot