Skip to content

UpSample optional kernel_size for deconv mode#5221

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

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

Conversation

@myron

@myron myron commented Sep 29, 2022

Copy link
Copy Markdown
Collaborator

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.

Signed-off-by: myron <amyronenko@nvidia.com>
Comment thread monai/networks/blocks/upsample.py Outdated
@KumoLiu

KumoLiu commented Sep 30, 2022

Copy link
Copy Markdown
Contributor

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

myron commented Sep 30, 2022

Copy link
Copy Markdown
Collaborator Author

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

myron and others added 3 commits September 30, 2022 16:50
Signed-off-by: myron <amyronenko@nvidia.com>
Signed-off-by: myron <amyronenko@nvidia.com>
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.

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

wyli commented Oct 1, 2022

Copy link
Copy Markdown
Contributor

/build

@wyli wyli enabled auto-merge (squash) October 1, 2022 13:24
@wyli wyli disabled auto-merge October 1, 2022 14:23
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli enabled auto-merge (squash) October 1, 2022 14:24
@wyli

wyli commented Oct 1, 2022

Copy link
Copy Markdown
Contributor

/build

@wyli wyli merged commit ffaa791 into Project-MONAI:dev Oct 1, 2022
@myron

myron commented Oct 3, 2022

Copy link
Copy Markdown
Collaborator Author

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

wyli commented Oct 3, 2022

Copy link
Copy Markdown
Contributor

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
### 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 <amyronenko@nvidia.com>
KumoLiu pushed a commit that referenced this pull request Nov 2, 2022
### 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 <amyronenko@nvidia.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
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.

4 participants