Skip to content

KeepLargest... by default use all labels#3776

Merged
wyli merged 5 commits intoProject-MONAI:devfrom
rijobro:KeepLargest_onehot
Feb 22, 2022
Merged

KeepLargest... by default use all labels#3776
wyli merged 5 commits intoProject-MONAI:devfrom
rijobro:KeepLargest_onehot

Conversation

@rijobro
Copy link
Copy Markdown
Contributor

@rijobro rijobro commented Feb 7, 2022 •
edited
Loading

  • We also required that the user define the applied labels at construction time. However, for many label segmentation, not all labels will necessarily always be present, and this causes an error. I think the default should be to apply to all present labels (calculated on the fly), but allow the user to limit to a subset of labels (as is the current implementation).
  • Improved testing
  • Harmonise implementations between FillHoles and KeepLargest...

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • 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.

@rijobro rijobro requested review from Nic-Ma and wyli February 7, 2022 15:53
@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Feb 7, 2022 •
edited
Loading

Hi @rijobro ,

Please check this PR #3647 and the associated ticket for the one-hot arg.

Thanks.

@rijobro rijobro force-pushed the KeepLargest_onehot branch from e5afdc5 to 1b8d5ca Compare February 8, 2022 09:45
@rijobro
Copy link
Copy Markdown
Contributor Author

rijobro commented Feb 8, 2022

@wyli @Nic-Ma I've reverted the modifications to one-hot in this PR.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Feb 8, 2022

thanks, this provides a good default for applied_labels, I'll merge once the unit tests are fixed. (torch.moveaxis is for torch 1.7+, our moveaxis is robust https://github.com/Project-MONAI/MONAI/blob/dev/monai/transforms/utils_pytorch_numpy_unification.py#L55)

@rijobro rijobro changed the title KeepLargest... determine if onehotted and labels KeepLargest... by default use all labels Feb 9, 2022
@rijobro
re-do for DCO
4b3de85 
Signed-off-by: Richard Brown <[email protected]>
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Feb 15, 2022 •
edited
Loading

Still there is a test issue, looks like the windows API requires a long into for onehot...

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Feb 22, 2022

/build

@wyli wyli enabled auto-merge (squash) February 22, 2022 17:09
@wyli wyli merged commit 3af9d73 into Project-MONAI:dev Feb 22, 2022
@rijobro rijobro deleted the KeepLargest_onehot branch February 23, 2022 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wyli wyli wyli approved these changes

@Nic-Ma Nic-Ma Awaiting requested review from Nic-Ma Nic-Ma is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rijobro @Nic-Ma @wyli