kornia
kornia copied to clipboard
WIP: Store GaussianBlur2d kernel as buffer
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?
I also modified a bug in test_module
which partially addresses #1505 but not fully.
@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.
hi @justinpinkney, have interest in finishing this one? now we don't have support to JIT on from kornia.filters
😄