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.

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

2679 Add IntensityStats transform to record intensity statistics into meta data #2685

Merged
merged 21 commits into from
Aug 4, 2021

Conversation

Nic-Ma
Copy link
Contributor

@Nic-Ma Nic-Ma commented Aug 3, 2021

Fixes #2679 .

Description

This PR added IntensityStats transform to record intensity statistics into meta data for other following transforms to dynamically adjust parameters for every image.

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.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Nic-Ma and others added 8 commits February 1, 2021 19:15
Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma Nic-Ma changed the title [WIP] 2679 Add IntensityStats transform to record intensity statistics into meta data 2679 Add IntensityStats transform to record intensity statistics into meta data Aug 3, 2021
@Nic-Ma Nic-Ma marked this pull request as ready for review August 3, 2021 10:17
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Aug 3, 2021

I enhanced RandShiftIntensityd transform to leverage the intensity stats at runtime, we can also enhance other intensity transforms later.

Thanks.

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

@wyli wyli left a comment

Choose a reason for hiding this comment

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

Thanks, I put some comments inline

monai/transforms/intensity/array.py Outdated Show resolved Hide resolved
monai/transforms/intensity/array.py Outdated Show resolved Hide resolved
monai/transforms/intensity/array.py Outdated Show resolved Hide resolved
monai/transforms/intensity/array.py Outdated Show resolved Hide resolved
Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Aug 3, 2021

Hi @wyli ,

Thanks for your review.
I updated the PR according to your comments.
Could you please help review it again?

Thanks in advance.

@Nic-Ma Nic-Ma requested a review from wyli August 3, 2021 15:20
Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

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

thanks, would be great to have the pickle example as a unit test, but I think it's beyond this PR.

Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Aug 3, 2021

Hi @wyli ,

I found an easy way to test the pickle issue, already added the logic in the unit test in the latest commit.
If we have self.test = lambda x: x in the __init__() of transform, it will raise the pickle error.

Thanks.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Aug 3, 2021

/black

monai-bot and others added 2 commits August 3, 2021 23:05
Signed-off-by: monai-bot <monai.miccai2019@gmail.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma Nic-Ma enabled auto-merge (squash) August 3, 2021 23:23
@Nic-Ma Nic-Ma merged commit 482ff5d into Project-MONAI:dev Aug 4, 2021
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.

Add IntensityStats transform
3 participants