Skip to content

5991 Support more spatial transforms to use lazy resampling#6080

Merged
wyli merged 65 commits intoProject-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 intoProject-MONAI:devfrom
yiheng-wang-nv:5991-add-spatial-transforms-for-lazy-resampling

Conversation

@yiheng-wang-nv
Copy link
Copy Markdown
Contributor

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

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
yiheng-wang-nv and others added 16 commits March 7, 2023 14:04
Signed-off-by: Yiheng Wang <[email protected]>
Signed-off-by: KumoLiu <[email protected]>
Signed-off-by: KumoLiu <[email protected]>
Signed-off-by: Yiheng Wang <[email protected]>
Signed-off-by: Yiheng Wang <[email protected]>
…hub.com:yiheng-wang-nv/MONAI into 5991-add-spatial-transforms-for-lazy-resampling

Signed-off-by: Yiheng Wang <[email protected]>
Signed-off-by: Yiheng Wang <[email protected]>
Signed-off-by: Yiheng Wang <[email protected]>
Signed-off-by: Yiheng Wang <[email protected]>
Signed-off-by: Yiheng Wang <[email protected]>
Signed-off-by: KumoLiu <[email protected]>
Signed-off-by: KumoLiu <[email protected]>
Signed-off-by: KumoLiu <[email protected]>
@yiheng-wang-nv yiheng-wang-nv requested a review from ericspod March 13, 2023 14:32
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Mar 13, 2023

/build

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Mar 13, 2023

/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 <[email protected]>
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Mar 13, 2023

/build

Signed-off-by: Wenqi Li <[email protected]>
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Mar 13, 2023

/build

Signed-off-by: Wenqi Li <[email protected]>
Copy link
Copy Markdown
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.

all tests look good now

Copy link
Copy Markdown
Contributor

@Nic-Ma Nic-Ma left a comment

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.

@wyli wyli force-pushed the 5991-add-spatial-transforms-for-lazy-resampling branch from 7215749 to efdc6c0 Compare March 14, 2023 16:18
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Mar 14, 2023

/build

@wyli wyli force-pushed the 5991-add-spatial-transforms-for-lazy-resampling branch from efdc6c0 to 589af32 Compare March 14, 2023 16:27
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Mar 14, 2023

/build

wyli added 2 commits March 14, 2023 17:39
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Mar 14, 2023

/build

@wyli wyli enabled auto-merge (squash) March 14, 2023 17:51
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Mar 14, 2023

/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