Skip to content

UpSample optional kernel_size for deconv mode#5221

Merged
wyli merged 8 commits intoProject-MONAI:devfrom
myron:upsample
Oct 1, 2022
Merged

UpSample optional kernel_size for deconv mode#5221
wyli merged 8 commits intoProject-MONAI:devfrom
myron:upsample

Conversation

@myron
Copy link
Copy Markdown
Collaborator

@myron myron commented Sep 29, 2022 •
edited by wyli
Loading

Description

Adds (optional) kernel_size parameter to UpSample, used for deconv (convolution transpose up-sampling).

This allows to upsample, e.g to upscale to 2x with a kernel_size 3. (currently the default is to upscale to 2x with a kernel size 2)

if this parameter is not set, the behavior is the same as before

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

@myron myron added this to the Auto3D Seg framework [P0 v1.0] milestone Sep 29, 2022
wyli
wyli reviewed Sep 29, 2022
View reviewed changes
@KumoLiu
Copy link
Copy Markdown
Contributor

KumoLiu commented Sep 30, 2022

Hi @myron, you can set up padding and output_padding separately like this, which might solve the test error of mismatched test output sizes.

output_padding = [0, ] * spatial_dims
for i, _scale_factor_ in enumerate(scale_factor_):
    if kernel_size_[i] % 2 == 0:
        output_padding[i] = _scale_factor_ - 2
    else:
        output_padding[i] = _scale_factor_ - 1

padding = tuple((i-1)//2 for i in kernel_size_)

self.add_module(
    "deconv",
    Conv[Conv.CONVTRANS, spatial_dims](
        in_channels=in_channels,
        out_channels=out_channels or in_channels,
        kernel_size=kernel_size_,
        stride=scale_factor_,
        padding=padding,
        output_padding=tuple(output_padding),
        bias=bias,
    ),
)

Thanks!

@myron
Copy link
Copy Markdown
Collaborator Author

myron commented Sep 30, 2022

ok, I believe I fixed that, and added another unit test. thanks you

wyli
wyli approved these changes Oct 1, 2022
View reviewed changes
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.

looks good to me, just to note that when the kernel size is not divisible by the scale_factor, this will introduce some checkerboard artefacts.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Oct 1, 2022

/build

@wyli wyli enabled auto-merge (squash) October 1, 2022 13:24
@wyli wyli disabled auto-merge October 1, 2022 14:23
@wyli
fixes downloading tests
b29add3 
Signed-off-by: Wenqi Li <[email protected]>
@wyli wyli enabled auto-merge (squash) October 1, 2022 14:24
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Oct 1, 2022

/build

@wyli wyli merged commit ffaa791 into Project-MONAI:dev Oct 1, 2022
@myron
Copy link
Copy Markdown
Collaborator Author

myron commented Oct 3, 2022

looks good to me, just to note that when the kernel size is not divisible by the scale_factor, this will introduce some checkerboard artefacts.

Thanks. is it a known fact? I use it with scale 2, kernel 3, I don't observe any artifacts.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Oct 3, 2022

the idea is presented here https://distill.pub/2016/deconv-checkerboard/, I guess it'll be less noticeable for segmentation tasks..

wyli pushed a commit that referenced this pull request Oct 10, 2022
@myron @wyli
UpSample optional kernel_size for deconv mode (#5221)
2687b72 
### Description

Adds (optional) kernel_size parameter to UpSample, used for deconv
(convolution transpose up-sampling).

This allows to upsample, e.g to upscale to 2x with a kernel_size 3.
(currently the default is to upscale to 2x with a kernel size 2)

if this parameter is not set, the behavior is the same as before


### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] 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).
- [x] 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.
- [x] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: myron <[email protected]>
KumoLiu pushed a commit that referenced this pull request Nov 2, 2022
@myron @KumoLiu
UpSample optional kernel_size for deconv mode (#5221)
8615ce8 
### Description

Adds (optional) kernel_size parameter to UpSample, used for deconv
(convolution transpose up-sampling).

This allows to upsample, e.g to upscale to 2x with a kernel_size 3.
(currently the default is to upscale to 2x with a kernel size 2)

if this parameter is not set, the behavior is the same as before


### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] 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).
- [x] 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.
- [x] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: myron <[email protected]>
Signed-off-by: KumoLiu <[email protected]>
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

Assignees

@myron myron

@wyli wyli

@Nic-Ma Nic-Ma

Labels

None yet

Projects

None yet

Milestone

Auto3D Seg framework [internal ongoin...

Development

Successfully merging this pull request may close these issues.

4 participants

@myron @KumoLiu @wyli @Nic-Ma