Use Ignite's interrupt api in MonaiAlgo by holgerroth · Pull Request #5071 · Project-MONAI/MONAI

holgerroth · 2022-09-01T23:42:14Z

Fixes #4554.

Requires latest Ignite RC.

Also improves ClientAlgo docstrings formatting.

Description

A few sentences describing the changes proposed in this pull request.

Status

Ready

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: Holger Roth <hroth@nvidia.com>

Nic-Ma · 2022-09-02T08:44:10Z

Hi @holgerroth @wyli ,

I think we should update the min_version of ignite in the monai_algo.py file to 0.4.10:
https://github.com/Project-MONAI/MONAI/blob/dev/monai/fl/client/monai_algo.py#L41
Other places keep the original min version.
Then when ignite released 0.4.10 next Monday, let's update the ignite version in all the docker and CI configs.
Now the min required version is 0.4.4 for all the places:
https://github.com/Project-MONAI/MONAI/blob/dev/monai/config/deviceconfig.py#L261
And the docker and CI version is 0.4.9:
https://github.com/Project-MONAI/MONAI/blob/dev/requirements-dev.txt#L3

What do you think?

Thanks.

Signed-off-by: Holger Roth <hroth@nvidia.com>

holgerroth · 2022-09-02T15:40:44Z

Hi @holgerroth @wyli ,

I think we should update the min_version of ignite in the monai_algo.py file to 0.4.10: https://github.com/Project-MONAI/MONAI/blob/dev/monai/fl/client/monai_algo.py#L41 Other places keep the original min version. Then when ignite released 0.4.10 next Monday, let's update the ignite version in all the docker and CI configs. Now the min required version is 0.4.4 for all the places: https://github.com/Project-MONAI/MONAI/blob/dev/monai/config/deviceconfig.py#L261 And the docker and CI version is 0.4.9: https://github.com/Project-MONAI/MONAI/blob/dev/requirements-dev.txt#L3

What do you think?

Thanks.

Actually, we don't need to import Ignite anymore in this class. I removed it.

Signed-off-by: Holger Roth <hroth@nvidia.com>

vfdev-5 · 2022-09-05T11:58:03Z

FYI, we just released 0.4.10, so you can check and also update MONAI deps.

Nic-Ma · 2022-09-05T14:56:25Z

FYI, we just released 0.4.10, so you can check and also update MONAI deps.

Cool!
@holgerroth Can I update the ignite related logic in your PR directly?

Thanks.

Signed-off-by: Nic Ma <nma@nvidia.com>

for more information, see https://pre-commit.ci

Nic-Ma

I have updated the ignite dependency in the PR.

Thanks.

Nic-Ma · 2022-09-05T15:27:25Z

/black

Nic-Ma · 2022-09-05T15:27:31Z

/build

Nic-Ma · 2022-09-05T15:28:16Z

Hi @wyli ,

Could you please help double confirm it?

Thanks in advance.

Nic-Ma · 2022-09-05T15:28:57Z

And @YanxuanLiu , may need your help to update the ignite version in the Blossom CI config to 0.4.10.

Thanks in advance.

Signed-off-by: monai-bot <monai.miccai2019@gmail.com>

Nic-Ma · 2022-09-06T00:15:16Z

/build

wyli · 2022-09-06T06:32:46Z

Hi @wyli ,

Could you please help double confirm it?

Thanks in advance.

I see there are docs-test errors: https://github.com/Project-MONAI/MONAI/runs/8197608122?check_suite_focus=true

Signed-off-by: Nic Ma <nma@nvidia.com>

Nic-Ma · 2022-09-06T07:30:44Z

/black

Nic-Ma · 2022-09-06T07:30:51Z

/build

Fixes Project-MONAI#4554. Requires latest Ignite RC. Also improves ClientAlgo docstrings formatting. ### Description A few sentences describing the changes proposed in this pull request. ### Status **Ready** ### Types of changes  - [x] 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: Holger Roth <hroth@nvidia.com> Signed-off-by: Nic Ma <nma@nvidia.com> Signed-off-by: monai-bot <monai.miccai2019@gmail.com> Co-authored-by: Nic Ma <nma@nvidia.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: monai-bot <monai.miccai2019@gmail.com>

holgerroth added 2 commits September 1, 2022 19:39

improve client algo docstring formatting

7ee19b6

Signed-off-by: Holger Roth <hroth@nvidia.com>

use ignite's new interrupt api

d48d1e8

Signed-off-by: Holger Roth <hroth@nvidia.com>

holgerroth added this to the Add MONAI FL client components [P0 v1.0] milestone Sep 1, 2022

holgerroth requested review from Nic-Ma and wyli September 1, 2022 23:42

Nic-Ma requested a review from vfdev-5 September 2, 2022 07:45

Nic-Ma reviewed Sep 2, 2022

View reviewed changes

Comment thread monai/fl/client/client_algo.py Outdated

Comment thread monai/fl/client/client_algo.py

remove ignite import

939decb

Signed-off-by: Holger Roth <hroth@nvidia.com>

add typing

bee31e3

Signed-off-by: Holger Roth <hroth@nvidia.com>

Nic-Ma and others added 3 commits September 5, 2022 23:02

Merge branch 'dev' into 4554-use-interrupt-api

93df384

[DLMED] update ignite support

41dcb61

Signed-off-by: Nic Ma <nma@nvidia.com>

[pre-commit.ci] auto fixes from pre-commit.com hooks

877f131

for more information, see https://pre-commit.ci

Nic-Ma approved these changes Sep 5, 2022

View reviewed changes

monai-bot and others added 2 commits September 5, 2022 15:32

[MONAI] code formatting

420a502

Signed-off-by: monai-bot <monai.miccai2019@gmail.com>

Merge branch 'dev' into 4554-use-interrupt-api

61628e2

vfdev-5 reviewed Sep 6, 2022

View reviewed changes

Comment thread monai/fl/client/client_algo.py

Nic-Ma added 3 commits September 6, 2022 15:09

[DLMED] fix docs issue

c932314

Signed-off-by: Nic Ma <nma@nvidia.com>

[DLMED] update according to comments

683cc58

Signed-off-by: Nic Ma <nma@nvidia.com>

[DLMED] remove abc

7ef6d0b

Signed-off-by: Nic Ma <nma@nvidia.com>

wyli enabled auto-merge (squash) September 6, 2022 09:07

wyli merged commit b6016a9 into Project-MONAI:dev Sep 6, 2022

Uh oh!

Conversation

holgerroth commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Status

Types of changes

Uh oh!

Uh oh!

Uh oh!

Nic-Ma commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

holgerroth commented Sep 2, 2022

Uh oh!

vfdev-5 commented Sep 5, 2022

Uh oh!

Nic-Ma commented Sep 5, 2022

Uh oh!

Nic-Ma left a comment

Choose a reason for hiding this comment

Uh oh!

Nic-Ma commented Sep 5, 2022

Uh oh!

Nic-Ma commented Sep 5, 2022

Uh oh!

Nic-Ma commented Sep 5, 2022

Uh oh!

Nic-Ma commented Sep 5, 2022

Uh oh!

Nic-Ma commented Sep 6, 2022

Uh oh!

wyli commented Sep 6, 2022

Uh oh!

Uh oh!

Nic-Ma commented Sep 6, 2022

Uh oh!

Nic-Ma commented Sep 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

holgerroth commented Sep 1, 2022 •

edited

Loading

Nic-Ma commented Sep 2, 2022 •

edited

Loading