-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Added GaussianBlur transform #2658
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
Conversation
Small linting errors.
|
@tejank10 thanks for the PR and sorry for delay. Finally I think the API we would like is with # functional.py
def gaussian_blur(img: Tensor, kernel_size: int, sigma: float) -> Tensor: The main problem is with PIL which can apply kernels larger than 5x5. Maybe, a solution can be to reuse torch implementation for that : About your implementation, I'll comment the things where I have questions. |
@tejank10 OK, let's work on this PR the following way:
I propose you to work on EDIT: I'll also push some modifications of |
Sounds good, will update the API. |
c43c414
to
02ad333
Compare
@yaox12 how about this API ? |
Codecov Report
@@ Coverage Diff @@
## master #2658 +/- ##
==========================================
+ Coverage 72.75% 72.90% +0.15%
==========================================
Files 96 96
Lines 8309 8396 +87
Branches 1292 1315 +23
==========================================
+ Hits 6045 6121 +76
- Misses 1871 1878 +7
- Partials 393 397 +4
Continue to review full report at Codecov.
|
@tejank10 we have to improve coverage by executing all input checkings and asserting that they raise correct exception. Can you work on that too ? |
@tejank10 do you plan to work on this PR this week ? Such that we do not do the same things without syncing. Otherwise, I'll rework everything tomorrow. |
Yeah, I'll be working on this PR this week. |
@tejank10 could you please push all your work today if any. I'll finalize today / tomorrow this PR. |
@tejank10 I'll fix broken CI and merge conflict today. Let me know if you want to push some other updates. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Added GaussianBlur transform * fixed linting * supports fixed radius for kernel * [WIP] New API for gaussian_blur * Gaussian blur with kernelsize and sigma API * Fixed implementation and updated tests * Added large input case and refactored gt into a file * Updated docs * fix kernel dimesnions order while creating kernel * added tests for exception handling of gaussian blur * fix linting, bug in tests * Fixed failing tests, refactored code and other minor fixes Co-authored-by: vfdev-5 <[email protected]>
* Added GaussianBlur transform * fixed linting * supports fixed radius for kernel * [WIP] New API for gaussian_blur * Gaussian blur with kernelsize and sigma API * Fixed implementation and updated tests * Added large input case and refactored gt into a file * Updated docs * fix kernel dimesnions order while creating kernel * added tests for exception handling of gaussian blur * fix linting, bug in tests * Fixed failing tests, refactored code and other minor fixes Co-authored-by: vfdev-5 <[email protected]>
This PR addresses issue #2635. It adds GaussianBlur image transformation. It takes a range of sigma as input. It picks a sigma for Gaussian kernel at random and applies the kernel to the image.
Let me know your thoughts @vfdev-5, @yaox12 on where could it be possibly improved.