Skip to content

5991 Support more spatial transforms to use lazy resampling - #6080

Merged
wyli merged 65 commits into
Project-MONAI:devfrom
yiheng-wang-nv:5991-add-spatial-transforms-for-lazy-resampling
Mar 14, 2023
Merged

5991 Support more spatial transforms to use lazy resampling#6080
wyli merged 65 commits into
Project-MONAI:devfrom
yiheng-wang-nv:5991-add-spatial-transforms-for-lazy-resampling

Conversation

@yiheng-wang-nv

@yiheng-wang-nv yiheng-wang-nv commented Feb 28, 2023

Copy link
Copy Markdown
Contributor

closes #5991 .

Description

Ready

Types of changes

  • 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.

@yiheng-wang-nv yiheng-wang-nv changed the title 5991 Support more spatial transforms to use lazy resampling [WIP] 5991 Support more spatial transforms to use lazy resampling Feb 28, 2023
Comment thread monai/transforms/spatial/array.py
wyli and others added 6 commits February 28, 2023 17:23
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Comment thread tests/test_resized.py
yiheng-wang-nv and others added 16 commits March 7, 2023 14:04
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
…hub.com:yiheng-wang-nv/MONAI into 5991-add-spatial-transforms-for-lazy-resampling

Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
@yiheng-wang-nv
yiheng-wang-nv requested a review from ericspod March 13, 2023 14:32
@wyli

wyli commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

/build

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli

wyli commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

/build

@wyli
wyli force-pushed the 5991-add-spatial-transforms-for-lazy-resampling branch from 715b042 to 3fb2d76 Compare March 13, 2023 17:53
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli

wyli commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

/build

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli

wyli commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

/build

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

@wyli wyli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

all tests look good now

@Nic-Ma Nic-Ma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the big update for lazy-resampling.
Put some comments inline.
Please double-confirm all the doc-strings.

Thanks.

mode: str = GridSampleMode.BILINEAR,
padding_mode: str = GridSamplePadMode.ZEROS,
align_corners: bool = False,
align_corners: bool = True,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

May I know why change the default value of align_corners?

Thanks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

there's a bug in the current dev implementation, the default behaviour of AffineTransform() is actually the same as AffineTransform(align_corners=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this change break any existing user programs? Or do we need to update any tutorial or bundle?
And I think better to use deprecated_arg_default? https://github.com/Project-MONAI/MONAI/blob/dev/monai/utils/deprecate_utils.py#L230?
CC @yiheng-wang-nv

Thanks.

Comment thread monai/transforms/spatial/array.py Outdated
Comment thread monai/transforms/spatial/functional.py Outdated
Comment thread monai/transforms/spatial/functional.py Outdated
Comment thread monai/transforms/spatial/functional.py Outdated
Comment thread monai/transforms/spatial/functional.py Outdated
wyli added 5 commits March 14, 2023 12:39
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli
wyli force-pushed the 5991-add-spatial-transforms-for-lazy-resampling branch from 7215749 to efdc6c0 Compare March 14, 2023 16:18
@wyli

wyli commented Mar 14, 2023

Copy link
Copy Markdown
Contributor

/build

…rms-for-lazy-resampling

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli
wyli force-pushed the 5991-add-spatial-transforms-for-lazy-resampling branch from efdc6c0 to 589af32 Compare March 14, 2023 16:27
@wyli

wyli commented Mar 14, 2023

Copy link
Copy Markdown
Contributor

/build

wyli added 2 commits March 14, 2023 17:39
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
…nsforms-for-lazy-resampling

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli

wyli commented Mar 14, 2023

Copy link
Copy Markdown
Contributor

/build

@wyli
wyli enabled auto-merge (squash) March 14, 2023 17:51
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli

wyli commented Mar 14, 2023

Copy link
Copy Markdown
Contributor

/build

@wyli
wyli merged commit 5e3b133 into Project-MONAI:dev Mar 14, 2023
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.

review and implement transform functionals in dev

4 participants