Added gaussian_blur_op #1450

ghosalsattam · 2020-03-27T04:20:26Z

This PR is aimed at putting a gaussian blur in an image. It reduces the noise in an image.
See Issue #1333

ghosalsattam · 2020-03-27T04:26:46Z

@seanpmorgan @gabrieldemarmiesse @WindQAQ @autoih Please provide a review

Squadrick · 2020-03-27T09:01:07Z

@ghosalsattam This is PR requires some more work to get to an acceptable state. I'll work you through the steps:

You shouldn't put the file in the base directory, put in under tensorflow_addons/image.
Update BUILD in tensorflow_addons/image.
The naming scheme of files should be file_name.py, not fileName.py.
You've implemented the op as an object, whereas it should actually be implemented as a function. See some of the other ops in tensorflow_addons/image to get a better idea.
You should use tf for the implementation instead of np. (ones, expand_dims).
All core TF ops should be used via the public API. So use tf.nn.convolution directly. Remove all tensorflow.python.* imports. This is because tensorflow.python.* imports are private.
The testing code should run using pytest.
The test should compare the output with other standard image processing libraries like PIL or sklearn-image.

ghosalsattam · 2020-03-27T13:30:01Z

@Squadrick I am trying to use tf.Tensor to find Gaussian Kernel.But when I am using the function assign in a variable of type tf.Variable I am getting the error 'tensorflow.python.framework.ops.EagerTensor' object has no attribute 'assign'
Can you please help me with the problem as I am new to tensorflow open source community.
Thank You

Squadrick · 2020-03-27T13:36:39Z

@ghosalsattam Can you post the code that's causing the error.

ghosalsattam · 2020-03-27T13:41:56Z

Ok

ghosalsattam · 2020-03-27T13:43:42Z

def findKernel(sigma,kSize,gaussianFilter):

	"This function creates a kernel of size [kSize*kSize]"
	for i in range(-kSize//2,kSize//2+1):
		for j in range(-kSize//2,kSize//2+1):
			gaussianFilter[i+kSize//2][j+kSize//2].assign(1/(2*np.pi*(sigma)**2)*np.exp(-(i**2+j**2)/(2*sigma**2)))
	print(gaussianFilter)
	return

gaussianFilter=tf.Variable(tf.zeros([5,5]))
findKernel(1,5,gaussianFilter)

ghosalsattam · 2020-03-27T13:54:36Z

@Squadrick Please help with the code I provided above

ghosalsattam · 2020-03-27T16:16:48Z

Ok I solved it. Thank you for the support and sorry for the inconvenience caused.

ghosalsattam · 2020-03-28T05:15:49Z

@Squadrick plaese provide a review.

remove averageSmoothing.py

ghosalsattam · 2020-03-28T06:16:01Z

@seanpmorgan @gabrieldemarmiesse @WindQAQ @autoih @Squadrick Please provide a review

ghosalsattam · 2020-03-28T13:11:27Z

Fixes Issue #1353

gabrieldemarmiesse · 2020-03-28T13:21:43Z

@ghosalsattam we usually provide review when all the tests in the CI are passing. Here you have some failed tests. You can look at the logs by clicking on "details".

rmlarsen · 2020-04-29T17:34:41Z

Somehow I cannot comment on that thread? Here is what I would like to say: "I believe @bhack is correct that this could be accomplished with two 1-dimensional convolutions, which would reduce the number of flops. However, if the kernel is small and the image large, reading the image twice might slow down the overall computation, compared to using a single Conv2D call. We should really have a specialized op for separable kernels like this. (Do we?)"

…

On Wed, Apr 29, 2020 at 10:25 AM Alexandre Passos ***@***.***> wrote: Ah, sorry, I misread the thread. @rmlarsen <https://github.com/rmlarsen> do you know who understands our depthwise conv kernels enough to answer this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1450 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEA72DUQWBZTWI4GSSMS2BTRPBPJLANCNFSM4LUXGCNA> .

rmlarsen · 2020-04-29T17:52:54Z

Correct me if I'm wrong, but from my experience, I think there are (at least) 3 distinct regimes for separable kernels (assuming a large image): 1. Tiny kernels: Use brute-force conv2d. 2. Medium kernels: Use "brute force" kernel, but take advantage of separability, one block at a time. 3. Large kernels: Use FFT. On Wed, Apr 29, 2020 at 10:34 AM Rasmus Munk Larsen <[email protected]> wrote:

…

Somehow I cannot comment on that thread? Here is what I would like to say: "I believe @bhack is correct that this could be accomplished with two 1-dimensional convolutions, which would reduce the number of flops. However, if the kernel is small and the image large, reading the image twice might slow down the overall computation, compared to using a single Conv2D call. We should really have a specialized op for separable kernels like this. (Do we?)" On Wed, Apr 29, 2020 at 10:25 AM Alexandre Passos < ***@***.***> wrote: > Ah, sorry, I misread the thread. > > @rmlarsen <https://github.com/rmlarsen> do you know who understands our > depthwise conv kernels enough to answer this? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1450 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AEA72DUQWBZTWI4GSSMS2BTRPBPJLANCNFSM4LUXGCNA> > . >

bhack · 2020-04-29T18:25:57Z

@rmlarsen I don't know if also the device specific knowledge it is going to play a role in the transform stack.

bhack · 2020-04-29T18:31:21Z

@rmlarsen P.s. I don't know if we have already this device specific knowledge or we need to waiting for something like a more advanced machine retargetability in MLIR

rmlarsen · 2020-04-29T18:34:33Z

Yes, the device will definitely play a role and, if nothing else, determine where the boundaries of the regimes are.

…

On Wed, Apr 29, 2020 at 11:31 AM bhack ***@***.***> wrote: @rmlarsen <https://github.com/rmlarsen> P.s. I don't know if we have already this device specific knowledge or we need to waiting for something like a more advanced machine retargetability in MLIR <https://llvm.discourse.group/t/rfc-enhancing-machine-retargetability-in-mlir/885> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1450 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEA72DTYKIVGATV6UVTUNWTRPBXAVANCNFSM4LUXGCNA> .

bhack · 2020-04-29T18:55:15Z

@rmlarsen @alextp Do you think it makes sense to track part of this in a new issue on the TF repo or is it not interesting enough for other use cases?

ghosalsattam · 2020-04-29T18:57:31Z

@bhack For now what should I do to this PR in order for it to be brought to merging state?

bhack · 2020-04-29T19:03:55Z

@ghosalsattam I think that you could use the 2d kernel here for merging and that you could track this on a DepthwiseConv2D specific issue on the TF repo after waiting for TF team feedback to see if the topic has a "general enough public".

ghosalsattam · 2020-04-29T19:05:48Z

And for CPU?

bhack · 2020-04-29T19:12:10Z

@ghosalsattam I think you can ignore the device and that it is better that DepthwiseConv2D will have directly this knowledge instead of having conditions here in the op so it knows how to propagate down the stack.
In proposal ticket on TF I hope that you could ask to pass the rank=1/separability arg directly (or something similar to inform it about the separate filter).

ghosalsattam · 2020-04-29T19:13:54Z

Ok!!!

…

On Thu, 30 Apr 2020 at 00:42, bhack ***@***.***> wrote: @ghosalsattam <https://github.com/ghosalsattam> I think you can ignore the device and that it is better that DepthwiseConv2D will have directly this knowledge instead of having conditions here in the op so it knows how to propagate down the stack. In proposal ticket on TF I hope that you could ask to pass the rank=1/separability arg directly (or something similar to inform it about the separate filter). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1450 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJMBF2CHLHIFYJ5HJS2QNMTRPB3ZXANCNFSM4LUXGCNA> .

ghosalsattam · 2020-04-29T19:55:28Z

@bhack Can it be merged now?

bhack · 2020-04-29T19:58:12Z

You need to wait for a reviewer with write power.

ghosalsattam · 2020-04-29T20:10:59Z

@WindQAQ Can it be merged now?

tensorflow_addons/image/filters.py

tensorflow_addons/image/tests/filters_test.py

ghosalsattam · 2020-04-30T09:45:03Z

@WindQAQ I have addressed all your changes. Can it be merged now?

bhack · 2020-04-30T09:59:52Z

@rmlarsen @alextp The feature request for Tensorflow is at tensorflow/tensorflow#39050. Please close it if you evaluate that it is not relevant enough.

ghosalsattam · 2020-04-30T14:20:31Z

@bhack According to me this should be implemented in all other conv ops where this situation may arise like along wiith depthwise conv operation- Conv2D, Conv3D,...
(If at all implementation is needed).

WindQAQ

Thanks again for the contribution.

stale

ghosalsattam · 2020-05-01T13:21:23Z

@ALL I learnt many new things from this contribution and cant wait for having new contributions. Thanx for your support.

rmlarsen · 2020-05-01T16:58:26Z

@ghosalsattam Thanks for the contribution!

* Gaussian Blur op * Adds Average smoothing op * Gaussian Filter * Delete averageSmoothing.py remove averageSmoothing.py * Delete averageSmoothing_test.py * Delete gaussianBlur_ops.py * Delete gaussianBlur_ops_test.py * Delete gaussianBlur_ops_test.py * improved test * improved test * added main * gaussian * test for gaussian Blur * after flake test * python black * Codeowners * BUILD * __init__ * gaussian_blur * ci test * github check * github check * github check * github test * github check * github check * github check * git check * github check * github check * check * check * ci check * check * test * check * test * github check * github check * ci test * github check * github check * github check * github check * github check * Changed the kernel computing technique * github test * github check * github check * github test * github check * Shifted to filters.py * changed codeowners * revert codeowners changes * changed codeowners * codeowners * Changed TO NHWC format * github check * Update filters.py * github check * Github Check * new commit * increased flexibility of kernel_size * added name parameter for the gaussian blur op * changed documentation * update __init__.py * Added 2D convolution Along with sequence of 1D convolution * Added experimental_compile * revert * github check * githun check

Gaussian Blur op

e27bd7f

googlebot added the cla: yes label Mar 27, 2020

Adds Average smoothing op

e1c1562

Squadrick added the image label Mar 27, 2020

Squadrick mentioned this pull request Mar 27, 2020

Average Smoothing #1451

Closed

Gaussian Filter

0eba6f2

ghosalsattam requested review from facaiy and WindQAQ as code owners March 27, 2020 21:08

ghosalsattam added 4 commits March 28, 2020 11:10

Delete averageSmoothing.py

ca68de6

remove averageSmoothing.py

Delete averageSmoothing_test.py

60be446

Delete gaussianBlur_ops.py

1c89c6b

Delete gaussianBlur_ops_test.py

725c3c2

ghosalsattam and others added 6 commits March 28, 2020 19:40

Delete gaussianBlur_ops_test.py

f6a952f

improved test

462f5db

Merge branch 'master' of https://github.com/ghosalsattam/addons

fc17a63

improved test

2b133f5

added main

494d4f8

gaussian

f82e21e

github check

cd9f5ed

WindQAQ requested changes Apr 30, 2020

View reviewed changes

tensorflow_addons/image/filters.py Show resolved Hide resolved

tensorflow_addons/image/tests/filters_test.py Show resolved Hide resolved

githun check

7b6bf24

bhack mentioned this pull request Apr 30, 2020

tf.nn.depthwise_conv2d with rank=1 kernels (separable filters) tensorflow/tensorflow#39050

Closed

WindQAQ self-requested a review April 30, 2020 18:36

WindQAQ approved these changes Apr 30, 2020

View reviewed changes

WindQAQ merged commit a6b4671 into tensorflow:master Apr 30, 2020

bhack mentioned this pull request Dec 23, 2020

Made blur faster by using gaussian decomposition property. #2315

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added gaussian_blur_op #1450

Added gaussian_blur_op #1450

ghosalsattam commented Mar 27, 2020 •

edited

Loading

ghosalsattam commented Mar 27, 2020

Squadrick commented Mar 27, 2020 •

edited by gabrieldemarmiesse

Loading

ghosalsattam commented Mar 27, 2020

Squadrick commented Mar 27, 2020

ghosalsattam commented Mar 27, 2020

ghosalsattam commented Mar 27, 2020 •

edited

Loading

ghosalsattam commented Mar 27, 2020

ghosalsattam commented Mar 27, 2020

ghosalsattam commented Mar 28, 2020

ghosalsattam commented Mar 28, 2020

ghosalsattam commented Mar 28, 2020

gabrieldemarmiesse commented Mar 28, 2020

rmlarsen commented Apr 29, 2020 via email

rmlarsen commented Apr 29, 2020 via email

bhack commented Apr 29, 2020

bhack commented Apr 29, 2020

rmlarsen commented Apr 29, 2020 via email

bhack commented Apr 29, 2020

ghosalsattam commented Apr 29, 2020

bhack commented Apr 29, 2020

ghosalsattam commented Apr 29, 2020

bhack commented Apr 29, 2020

ghosalsattam commented Apr 29, 2020 via email

ghosalsattam commented Apr 29, 2020 •

edited

Loading

bhack commented Apr 29, 2020

ghosalsattam commented Apr 29, 2020

ghosalsattam commented Apr 30, 2020

bhack commented Apr 30, 2020

ghosalsattam commented Apr 30, 2020 •

edited

Loading

WindQAQ left a comment

ghosalsattam commented May 1, 2020

rmlarsen commented May 1, 2020

Added gaussian_blur_op #1450

Added gaussian_blur_op #1450

Conversation

ghosalsattam commented Mar 27, 2020 • edited Loading

ghosalsattam commented Mar 27, 2020

Squadrick commented Mar 27, 2020 • edited by gabrieldemarmiesse Loading

ghosalsattam commented Mar 27, 2020

Squadrick commented Mar 27, 2020

ghosalsattam commented Mar 27, 2020

ghosalsattam commented Mar 27, 2020 • edited Loading

def findKernel(sigma,kSize,gaussianFilter):

ghosalsattam commented Mar 27, 2020

ghosalsattam commented Mar 27, 2020

ghosalsattam commented Mar 28, 2020

ghosalsattam commented Mar 28, 2020

ghosalsattam commented Mar 28, 2020

gabrieldemarmiesse commented Mar 28, 2020

rmlarsen commented Apr 29, 2020 via email

rmlarsen commented Apr 29, 2020 via email

bhack commented Apr 29, 2020

bhack commented Apr 29, 2020

rmlarsen commented Apr 29, 2020 via email

bhack commented Apr 29, 2020

ghosalsattam commented Apr 29, 2020

bhack commented Apr 29, 2020

ghosalsattam commented Apr 29, 2020

bhack commented Apr 29, 2020

ghosalsattam commented Apr 29, 2020 via email

ghosalsattam commented Apr 29, 2020 • edited Loading

bhack commented Apr 29, 2020

ghosalsattam commented Apr 29, 2020

ghosalsattam commented Apr 30, 2020

bhack commented Apr 30, 2020

ghosalsattam commented Apr 30, 2020 • edited Loading

WindQAQ left a comment

Choose a reason for hiding this comment

ghosalsattam commented May 1, 2020

rmlarsen commented May 1, 2020

ghosalsattam commented Mar 27, 2020 •

edited

Loading

Squadrick commented Mar 27, 2020 •

edited by gabrieldemarmiesse

Loading

ghosalsattam commented Mar 27, 2020 •

edited

Loading

ghosalsattam commented Apr 29, 2020 •

edited

Loading

ghosalsattam commented Apr 30, 2020 •

edited

Loading