Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add generic kernel transform with support for multiple kernels #5317

Merged
merged 23 commits into from
Jan 4, 2023
Merged

Add generic kernel transform with support for multiple kernels #5317

merged 23 commits into from
Jan 4, 2023

Conversation

kbressem
Copy link
Contributor

@kbressem kbressem commented Oct 12, 2022

Signed-off-by: kbressem [email protected]

Fixes #3178

Description

As discussed in #3178, this PR adds a more general interface to apply simple convolutions to images. This PR (re-)implements 2D and 3D versions for mean filtering, edge detection, outline detection, mask dilatation and image sharpening.

Detailed description

The main transform is ImageFilter. It comes with some preset filters that can be initialized using a string. Available presets are ["mean", "laplace", "elliptical", "sobel", "sharpen", "median", "gauss", "savitzky_golay"]. For example, a transformation for mean filtering can be created with:

mean_filter = ImageFilter("mean", 3)

ImageFilter automatically detects whether the input is 2D or 3D and creates the appropriate filter on the first call. This filter will then be used in future calls of the transformation, so switching between 2D and 3D images is not supported.

The initialization form string is only a convenience function. All preset filters are also available in monai.networks.layers.simplelayers as nn.Module subclasses. ImageFiltercan also be created directly from annn.Module`. For example, to create a transformation for mean filtering, the following is also possible:

filter_3d = MeanFilter(spatial_dims=3, size=3)
mean_filter = ImageFilter(filter_3d)

In this example, however, the filter size is predefined and is not automatically derived from the input.
In addition, it is also possible to use custom filters with ImageFilter using a torch.Tensor or numpy.ndarray. For example:

filter_3d = torch.ones(3,3,3)
mean_filter = ImageFilter(filter_3d)

RandImageFilter is a randomizable variant of ImageFilter that executes with probability p on each call. Both ImageFilter and RandImageFilter also have dictionary versions.

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.

@kbressem kbressem marked this pull request as ready for review October 12, 2022 20:58
@wyli
Copy link
Contributor

wyli commented Oct 13, 2022

thanks, this looks good to me, could you please address the minor issues reported here? https://deepsource.io/gh/Project-MONAI/MONAI/run/76df1e85-7eb3-4b7e-8073-088d5f5ee6ee/python/ ?

more specifically it might be useful to make the kernel functions classmethods:

class KernelTransform:
    ...
    @classmethod
    def laplacian_kernel(cls, size, ndim) -> torch.Tensor:
        ...
        return kernel

then we could reuse the kernel elsewhere (without creating a KernelTransform instance):

KernelTransform.laplacian_kernel(..)

@kbressem
Copy link
Contributor Author

thanks, this looks good to me, could you please address the minor issues reported here?

Thanks @wyli. I will replace the assertions with proper errors and also convert all kernel functions to classmethods.

One question though, I have not implemented the Gaussian kernel as it takes additional arguments (sigma and erf) and I was not sure how to go about it. I could use **kwargs and pass them to the function, but that way you wouldn't be able to see what arguments are required for the Gaussian kernel. I could also add sigma and erf to the arguments in __init__, though they are not needed for most kernels. What are your thoughts on this?

Also, when adding the new transforms to __all__ in the array.py and to the transforms.rst I noticed that the items are not sorted alphabetically. I found this a bit unclear, shall I update the order in this PR or raise an issue?

@wyli
Copy link
Contributor

wyli commented Oct 13, 2022

thank you, for the first issue I think it'll be good to use kwargs, and in the docstring explain that the corresponding required arguments are described in each create_[name]_kernel function (or saying k_size will be interpreted differently by each create kernel function)...

also precompute the kernel func would save a few cycles during __call__:

   def __init__(self, kernel, spatial_dim, k_size, dtype, **kernel_args):
       if isinstance(kernel, str):
           func = self._create_kernel_from_string(name)
       self.kernel = func(spatial_dim, k_size, **kernel_args)
       self.dtype = dtype

  def __call__(self, ...):
      self.kernel.to(device=input.device, dtype=self.dtype or input.dtype)

for the 2nd issue, would be great to have a second issue/PR...

@kbressem
Copy link
Contributor Author

also precompute the kernel func would save a few cycles during __call__:

The reason this is not done, is that I do not know if the image will be 2D or 3D until the function is called (see line 1564 in the array.py, where I extract the ndim from the image).

However, I could precompute 2D and 3D kernels and just use the appropriate kernel at runtime?

thank you, for the first issue I think it'll be good to use kwargs, and in the docstring explain that the corresponding required arguments are described in each create_[name]_kernel function (or saying k_size will be interpreted differently by each create kernel function)...

Ok, then I will use kwargs and add the information to the main docstring as well as the each create_[name]_kernel function

@wyli
Copy link
Contributor

wyli commented Oct 13, 2022

However, I could precompute 2D and 3D kernels and just use the appropriate kernel at runtime?

sure, i think that's a nice idea

@ericspod
Copy link
Member

I would suggest putting the kernel creation functions into a utilities file in transforms or networks as regular functions so that they are accessible to other if we wanted to use them for other purposes.

@kbressem kbressem marked this pull request as draft October 22, 2022 20:58
@kbressem kbressem changed the title Add generic kernel transform with support for multiple kernels 🚧 WIP: Add generic kernel transform with support for multiple kernels Oct 23, 2022
kbressem and others added 19 commits November 14, 2022 20:48
- rename class to `ImageFilter`
- `ImageFilter` now accepts strings (preset filters), `torch.Tensor` or `np.ndarray`, `nn.Module` and `monai.transforms.Transform`
- The filter is created just once, to make `__call__` faster
- Additional `nn.Modules` for simple filters have been created to make them easier available
- add tests for `ImageFilter` and `MeanFilter`

- Extend tests for ImageFilter
- Maybe extend ImageFilter to accept multiple arguments?
- Add Tests for other nn.Modules

Signed-off-by: kbressem <[email protected]>
Signed-off-by: kbressem <[email protected]>
Signed-off-by: kbressem <[email protected]>
Signed-off-by: kbressem <[email protected]>
Signed-off-by: kbressem <[email protected]>
Signed-off-by: kbressem <[email protected]>
@kbressem kbressem changed the title 🚧 WIP: Add generic kernel transform with support for multiple kernels Add generic kernel transform with support for multiple kernels Dec 31, 2022
@kbressem kbressem marked this pull request as ready for review January 1, 2023 10:56
@kbressem
Copy link
Contributor Author

kbressem commented Jan 1, 2023

@wyli @ericspod I've updated the PR incorporating your suggestions. Sorry for the delay.

There an issue remaining with make html failing, but I can't reproduce it locally. Could you help me with it, please? Seems to be related to #5793.

@wyli
Copy link
Contributor

wyli commented Jan 4, 2023

/build

Copy link
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.

Thanks, it looks good to me.

@wyli wyli enabled auto-merge (squash) January 4, 2023 16:44
@wyli wyli merged commit c5ea694 into Project-MONAI:dev Jan 4, 2023
@kbressem kbressem deleted the 3178-generic-filterkernel-transform branch January 4, 2023 20:33
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.

Antialiasing for labels
3 participants