[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

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

Open
bhack opened this issue Apr 30, 2020 · 7 comments
Open

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

bhack opened this issue Apr 30, 2020 · 7 comments
Assignees
@rohan100jain
Labels
comp:apis Highlevel API related issues type:feature Feature requests

Comments

@bhack
Copy link
Contributor
bhack commented Apr 30, 2020

Please make sure that this is a feature request. As per our GitHub Policy, we only address code/doc bugs, performance issues, feature requests and build/installation issues on GitHub. tag:feature_template

System information

  • TensorFlow version (you are using):
    nigthly
  • Are you willing to contribute it (Yes/No):
    no

Describe the feature and the current behavior/state.
There is a thread about twice calling DepWhiseConv2d TF ops when we have rank=1 kernels (separable filters) VS a single call to DepWhiseConv2d with 2d kernel.
I suppose that is would be hard to fuse on trace two DepWhiseConv2d on the separable filter.
So I was asking why with the current DepWhiseConv2d in TF doesn't handle the kernel rank=1 case with an parameter for down the stack compiler/transformations (kernel size, input size, device type, etc.).

Will this change the current api? How?
Yes
Who will benefit with this feature?
Separable filters (gaussian blur, etc)
Any Other info.
See tensorflow/addons#1450 (comment) with follow-up @alextp and @rmlarsen comments.
@bhack bhack added the type:feature Feature requests label Apr 30, 2020
@bhack bhack mentioned this issue Apr 30, 2020
Merged
@amahendrakar amahendrakar added the comp:apis Highlevel API related issues label Apr 30, 2020
@amahendrakar amahendrakar assigned ymodak and unassigned amahendrakar Apr 30, 2020
@ghosalsattam
Copy link

@bhack @alextp @rmlarsen 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)

@ymodak ymodak assigned rohan100jain and unassigned ymodak May 12, 2020
@bhack
Copy link
Contributor Author
bhack commented Jul 24, 2020

/cc @jpienaar

@jpienaar
Copy link
Member

Is the proposal here that DepthwiseConv2dNative (for different dimensions) has an additional attribute to say that is separable and in which case it gets a rank 1 kernel. And allows backends to decide whether they want to implement that as a single call with 2D kernel or 2 calls of DepthwiseConv2dNative with said rank 1 kernel? The rationale being that matching these two calls would be too difficult/fragile?

So it isn't a case where one would have

%5 = tf.SomeKernelGeneration()
%10 = DepthwiseConv2dNative(..., %5)
%20 = DepthwiseConv2dNative(%10 ..., %5)

and need to find cases where you have 1 dephtwise conv feeding into a next and they both share the same kernel, and if so one can convert to the other call form?

@bhack
Copy link
Contributor Author
bhack commented Jul 27, 2020

I think that the framework can analyze if the kernel Is rank==1.
Then down to the stack It can see if it wants to separate the kernel reasoning on different parameters (input size, kernel size, hardware factors etc..).

@bhack bhack mentioned this issue Dec 23, 2020
Closed
1 task
@bhack bhack mentioned this issue Feb 20, 2022
Closed
@bhack
Copy link
Contributor Author
bhack commented Feb 20, 2022 •
edited
Loading

Any news on this? We are adding a Gaussian filter again in Keras-cv:

keras-team/keras-cv#143

@gadagashwini
Copy link
Contributor

Hi @bhack, Are you looking similar feature tfa.image.gaussian_filter2d and tfio.experimental.filter.gaussian. Thank you!

@gadagashwini gadagashwini added the stat:awaiting response Status - Awaiting response from author label Jun 22, 2022
@bhack
Copy link
Contributor Author
bhack commented Jun 22, 2022

No, I meant these kind of optimizations could be handled by XLA/MLIR

@google-ml-butler google-ml-butler bot removed the stat:awaiting response Status - Awaiting response from author label Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rohan100jain rohan100jain

Labels
comp:apis Highlevel API related issues type:feature Feature requests
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@rohan100jain @jpienaar @bhack @ghosalsattam @ymodak @amahendrakar @gadagashwini