Fix generalized dice computation #7970

surajpaib · 2024-07-31T21:41:58Z

Description

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

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: Suraj Pai <bspai@bwh.harvard.edu> Similar functionality to torchmetrics Update Lint and update sum_over_labels Update docstring Update docstring

monai/metrics/generalized_dice.py

KumoLiu

Thanks for the contribution, overall looks good to me.
Once you finished the PR, please let me know, I will add more people help review it, thanks.

monai/metrics/generalized_dice.py

KumoLiu · 2024-08-26T05:27:59Z

Hi @surajpaib, do you plan to address the comments on the PR? I’m hoping we can get this merged into version 1.4, but we only have about one or two weeks left to add new features.

surajpaib · 2024-08-27T14:20:08Z

Hi @KumoLiu, sorry for the gap, was away on vacation. I think I can work on this in the next few days and get it in.

surajpaib · 2024-08-28T00:23:11Z

@KumoLiu I've incorporated the comments we discussed.

deprecated_arg_default is added to the initialization and for the reduction, I've added deprecated_arg.

The tests seem to be failing because of the version string - I assume we can set this appropriately during the release.

Let me know if you see any other changes

monai/metrics/generalized_dice.py

Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com> Signed-off-by: Suraj Pai <b.pai@maastrichtuniversity.nl>

monai/metrics/generalized_dice.py

Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com> Signed-off-by: Suraj Pai <b.pai@maastrichtuniversity.nl>

monai/metrics/generalized_dice.py

Signed-off-by: Suraj Pai <bspai@bwh.harvard.edu>

KumoLiu

Thanks for the update, LGTM.
Could you please help fix the ci issue and dco issue then I can help trigger more tests and merge this one?
https://github.com/Project-MONAI/MONAI/actions/runs/10733660015/job/29767399315?pr=7970#step:7:428

monai/metrics/generalized_dice.py

tests/test_compute_generalized_dice.py

Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com> Signed-off-by: Suraj Pai <b.pai@maastrichtuniversity.nl>

KumoLiu · 2024-09-06T14:38:05Z

Hi @surajpaib, you can fix the dco issue by following the method here: https://github.com/Project-MONAI/MONAI/pull/7970/checks?check_run_id=29787940944
Also please update here: https://github.com/Project-MONAI/MONAI/pull/7970/files#r1746799238
Then I will try to merge this one, thanks!

_________ DCO Remediation Commit for Suraj Pai <bspai@bwh.harvard.edu> I, Suraj Pai <bspai@bwh.harvard.edu>, hereby add my Signed-off-by to this commit: 2081cd2 I, Suraj Pai <bspai@bwh.harvard.edu>, hereby add my Signed-off-by to this commit: 3805fea I, Suraj Pai <bspai@bwh.harvard.edu>, hereby add my Signed-off-by to this commit: fb6e660 I, Suraj Pai <bspai@bwh.harvard.edu>, hereby add my Signed-off-by to this commit: 8274060 Signed-off-by: Suraj Pai <bspai@bwh.harvard.edu>

Signed-off-by: Suraj Pai <bspai@bwh.harvard.edu>

KumoLiu · 2024-09-07T07:14:20Z

/build

Fixes Project-MONAI#7966 ### Description A few sentences describing the changes proposed in this pull request. ### 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: Suraj Pai <b.pai@maastrichtuniversity.nl> Signed-off-by: Suraj Pai <bspai@bwh.harvard.edu> Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>

Fix generalized dice computation

123c778

Signed-off-by: Suraj Pai <bspai@bwh.harvard.edu> Similar functionality to torchmetrics Update Lint and update sum_over_labels Update docstring Update docstring

KumoLiu reviewed Aug 1, 2024

View reviewed changes

monai/metrics/generalized_dice.py Outdated Show resolved Hide resolved

Merge branch 'Project-MONAI:dev' into dev

c936162

KumoLiu reviewed Aug 7, 2024

View reviewed changes

monai/metrics/generalized_dice.py Show resolved Hide resolved

monai/metrics/generalized_dice.py Outdated Show resolved Hide resolved

ericspod reviewed Aug 7, 2024

View reviewed changes

monai/metrics/generalized_dice.py Outdated Show resolved Hide resolved

Merge branch 'gen_dice' into dev

d16bf41

surajpaib and others added 5 commits August 27, 2024 10:29

Merge branch 'Project-MONAI:dev' into dev

e8db89e

Merge branch 'dev' of github.com:surajpaib/MONAI into dev

61bbb60

Add updates from comments

2081cd2

Minor doc fixes

3805fea

Update version string

fb6e660

surajpaib marked this pull request as ready for review September 4, 2024 20:39

surajpaib requested a review from KumoLiu September 4, 2024 20:40

KumoLiu reviewed Sep 5, 2024

View reviewed changes

monai/metrics/generalized_dice.py Outdated Show resolved Hide resolved

Update monai/metrics/generalized_dice.py

4ea6a2c

Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com> Signed-off-by: Suraj Pai <b.pai@maastrichtuniversity.nl>

KumoLiu reviewed Sep 5, 2024

View reviewed changes

monai/metrics/generalized_dice.py Outdated Show resolved Hide resolved

Update monai/metrics/generalized_dice.py

2bc6dd8

Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com> Signed-off-by: Suraj Pai <b.pai@maastrichtuniversity.nl>

KumoLiu reviewed Sep 5, 2024

View reviewed changes

monai/metrics/generalized_dice.py Outdated Show resolved Hide resolved

surajpaib and others added 3 commits September 5, 2024 13:10

Update reduction for channel cases

8274060

Fix line length

2e7e04c

Signed-off-by: Suraj Pai <bspai@bwh.harvard.edu>

Merge branch 'dev' into dev

cfbeb0e

KumoLiu approved these changes Sep 6, 2024

View reviewed changes

monai/metrics/generalized_dice.py Outdated Show resolved Hide resolved

tests/test_compute_generalized_dice.py Show resolved Hide resolved

Update monai/metrics/generalized_dice.py

72ed145

Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com> Signed-off-by: Suraj Pai <b.pai@maastrichtuniversity.nl>

surajpaib added 3 commits September 6, 2024 11:38

Merge branch 'dev' of github.com:surajpaib/MONAI into dev

bf0c5f0

Signed-off-by: Suraj Pai <bspai@bwh.harvard.edu>

Fix flake8 error

27a0c58

Signed-off-by: Suraj Pai <bspai@bwh.harvard.edu>

Merge branch 'dev' into dev

c704cc8

KumoLiu merged commit d02ba11 into Project-MONAI:dev Sep 7, 2024
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix generalized dice computation #7970

Fix generalized dice computation #7970

surajpaib commented Jul 31, 2024

KumoLiu left a comment

KumoLiu commented Aug 26, 2024

surajpaib commented Aug 27, 2024

surajpaib commented Aug 28, 2024

KumoLiu left a comment

KumoLiu commented Sep 6, 2024

KumoLiu commented Sep 7, 2024

Fix generalized dice computation #7970

Fix generalized dice computation #7970

Conversation

surajpaib commented Jul 31, 2024

Description

Types of changes

KumoLiu left a comment

Choose a reason for hiding this comment

KumoLiu commented Aug 26, 2024

surajpaib commented Aug 27, 2024

surajpaib commented Aug 28, 2024

KumoLiu left a comment

Choose a reason for hiding this comment

KumoLiu commented Sep 6, 2024

KumoLiu commented Sep 7, 2024