Skip to content

9 intensity normalisation transform#25

Merged
ericspod merged 4 commits intomasterfrom
9-intensity-normalisation-transform
Jan 22, 2020
Merged

9 intensity normalisation transform#25
ericspod merged 4 commits intomasterfrom
9-intensity-normalisation-transform

Conversation

@Nic-Ma
Copy link
Copy Markdown
Contributor

@Nic-Ma Nic-Ma commented Jan 14, 2020 •
edited
Loading

Hi All,

This PR implemented the intensity normalization transform for task #9
Designed according to our latest discussion:
1. input data is dict format with keys for fields.
2. only based on PyTorch and data shape is channel_last.

Could you please help review it when you are available?
Thanks in advance.

I verified this PR locally with the below program:

# test 1 key with auto subtrahend and divisor
normalizer = IntensityNormalizer(apply_keys=('image',))
img = np.arange(27).reshape(3, 3, 3).astype(np.float32)
label = np.arange(27).reshape(3, 3, 3).astype(np.float32)
data = {'image': img, 'label': label}
print(data)
print(normalizer(data=data))

# test 2 keys with specified subtrahend and divisor
normalizer = IntensityNormalizer(
    apply_keys=('image', 'label'),
    subtrahend=np.ones((3), dtype=np.float32),
    divisor=np.ones((3), dtype=np.float32)
)
img = np.arange(27).reshape(3, 3, 3).astype(np.float32)
label = np.arange(27).reshape(3, 3, 3).astype(np.float32)
data = {'image': img, 'label': label}
print(data)
print(normalizer(data=data))

@Nic-Ma Nic-Ma mentioned this pull request Jan 14, 2020
Closed
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Jan 20, 2020

please update based on #9 (comment)

@Nic-Ma
[DLMED] implement intensity normalization transform
41fd036 
design according to our latest discussion:
1. input data is dict format with keys for fields.
2. only based on PyTorch and data shape is channel_last.
@Nic-Ma Nic-Ma force-pushed the 9-intensity-normalisation-transform branch from 8cd6e33 to 41fd036 Compare January 21, 2020 05:02
@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Jan 21, 2020

Hi @wyli @ericspod @atbenmurray ,

I already update the code and PR description according to our latest discussion.
Could you please help review it again?
Thanks in advance.

wyli and others added 2 commits January 21, 2020 23:16
@wyli @ericspod
@IsaacYangSLA @Nic-Ma
9 part a adding test intensity normalisation transform (#33)
e31ec10 
* Adding script to run unit tests and example test cases (#29)

Adding script to run unit tests and example test cases

* initial unit tests for dice loss (#27)

* initial unit tests for 2d/3d unet
* unit tests update
 - triggering unit tests via github workflow
 - renamed testconvolutions.py to test_convolutions.py
 - test unet test cases as variables for readability

* initial unit tests for 2d/3d unet (#26)

* initial unit tests for 2d/3d unet
* unit tests update
 - triggering unit tests via github workflow
 - renamed testconvolutions.py to test_convolutions.py
 - test unet test cases as variables for readability

* 14 code examples of monai input data pipeline (#24)

* fixes cardiac example

* update example cardiac segmentation

* Create .gitlab-ci.yml (#30)

an initial step towards #19

* tests intensity normalizer

- revised to support both `[key]` and `key` as an input for apply_keys
- added `NumpyImageTestCase2D` and `TorchImageTestCase2D`

* style updates and new test cases:

- adding copyright notice
- validate user input before setting class member
- one line space after copyright
- testing multiple keys input data

Co-authored-by: Eric Kerfoot <[email protected]>
Co-authored-by: Isaac Yang <[email protected]>
@wyli
Merge branch 'master' into 9-intensity-normalisation-transform
70e675d
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Jan 21, 2020

@ericspod could you review this PR? mainly added a minimal transform interface, and updated ImageTestCase to have NumpyImageTestCase2D and TorchImageTestCase2D .

@wyli wyli removed their request for review January 21, 2020 15:35
@wyli wyli mentioned this pull request Jan 21, 2020
Merged
@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Jan 22, 2020

Hi @ericspod @atbenmurray @wyli ,

I updated this PR to remove dict-key processing and be compatible with Eric's other transforms.
Could you please help review again?
Thanks in advance.

@Nic-Ma Nic-Ma force-pushed the 9-intensity-normalisation-transform branch from 2d9c037 to a2a4e23 Compare January 22, 2020 01:18
ericspod
ericspod approved these changes Jan 22, 2020
View reviewed changes
tests/test_intensity_normalizer.py
self.assertTrue(np.allclose(normalised, expected))


if __name__ == '__main__':
Copy link
Copy Markdown
Member

@ericspod ericspod Jan 22, 2020

Choose a reason for hiding this comment

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

Generally you don't need a main section in the test case scripts, you can run tests from the root directory with

python -m unittest test/test_intensity_normalizer.py

It doesn't hurt to be here though.

Copy link
Copy Markdown
Contributor

@wyli wyli Jan 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Good point! @atbenmurray could you please update the contribution guidelines about running all unit tests and single unit test . PR for new features should include new unit tests and inherit test case base classes.

@ericspod
Copy link
Copy Markdown
Member

ericspod commented Jan 22, 2020

Beyond the issue with the CI pipeline not completing the code is good to go.

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Jan 22, 2020

Beyond the issue with the CI pipeline not completing the code is good to go.

Hi @ericspod ,

Thanks for your quick review!
I think maybe gitlab-staging server is unavailable now.
The gitlab-master server passed tests.
Of course, I am OK if you think it's better to wait for the stage server recovery.
Thanks.

@ericspod
Copy link
Copy Markdown
Member

ericspod commented Jan 22, 2020

I would just go ahead with the merge.

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Jan 22, 2020

I would just go ahead with the merge.

Cool, thanks!
Could you please help merge it directly?

@ericspod ericspod merged commit 1390701 into master Jan 22, 2020
@wyli wyli deleted the 9-intensity-normalisation-transform branch April 6, 2020 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wyli wyli wyli left review comments

@ericspod ericspod ericspod approved these changes

@atbenmurray atbenmurray Awaiting requested review from atbenmurray

@yanchengnv yanchengnv Awaiting requested review from yanchengnv

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

@Nic-Ma @wyli @ericspod