kornia icon indicating copy to clipboard operation
kornia copied to clipboard

WIP: Store GaussianBlur2d kernel as buffer

Open justinpinkney opened this issue 2 years ago • 2 comments

Changes

Precomputes the kernels for GaussianBlur2d module and stores as buffers.

Fixes #1489

Discussion

  • This is a straightforward change to store the kernel as a buffer only for GaussianBlur2d. This same change is actually relevant for many other filters, do we want to also update those at the same time? Some would be equally straightforward, some probably not (e.g. canny)
  • Changing the module properties will not change the behaviour (see example below). This is the same behaviour as other module in PyTorch (e.g. Conv2d, you can set kernel_size but it has no effect), so users will hopefully not be suprrised by this behaviour, but is this considered ok?
  • There was a mention of adding a recommendation in the documentation about using module vs function version. Where would the appropriate place be for this? Presumably if many filters are updated a more generic statement on "module vs function" would be useful, rather than having the same note in every docstring.
module = GaussianBlur2d([3,3], [1.5, 1.5])
module.kernel_size = [7, 7]
# module will still use kernel_size = [3,3]

Type of change

  • [x] 🔬 New feature (non-breaking change which adds functionality)
  • [ ] 📝 This change requires a documentation update (maybe)

Checklist

  • [ ] My code follows the style guidelines of this project
  • [ ] I have performed a self-review of my own code
  • [ ] I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] My changes generate no new warnings
  • [ ] Did you update CHANGELOG in case of a major change?

justinpinkney avatar Dec 17 '21 10:12 justinpinkney

I also modified a bug in test_module which partially addresses #1505 but not fully.

justinpinkney avatar Dec 22 '21 16:12 justinpinkney

@justinpinkney been playing around with this and we need to find a workaround for torchscript. I added a small test to better understand the module behaviour.

edgarriba avatar Jan 30 '22 17:01 edgarriba

hi @justinpinkney, have interest in finishing this one? now we don't have support to JIT on from kornia.filters 😄

johnnv1 avatar Mar 04 '23 14:03 johnnv1