Repository navigation
Consider removing window_reversal from ConvolutionOp #1181
Description
I've recently realized that I'm not sure if window_reversal is actually used by frontends. E.g. JAX doesn't use it, and cursory inspection of TF/XLA suggests that it doesn't use it either. If that checks out, we should remove it from the spec to make things simpler. Related issue for ReduceWindowOp: #585.
Activity
loganchien commentedon Feb 24, 2023
AFAIK,
window_reversalis being used in the lowering of transposed convolution.sdasgup3 commentedon Mar 8, 2023
Thanks @loganchien for the valuable input. We will take that into account.
loganchien commentedon Mar 8, 2023
Conceptually, Transposed Convolution (a.k.a.
Deconvolution,Conv2dBackpropInput,tf.nn.atrous_conv2d) is the inverse of Convolution. Its computation can be translated intoConv(x, Reverse(filter)).TF2Xla implement this logic by inserting a RevOp:
https://github.com/tensorflow/tensorflow/blob/59fc820f7d6383e38ce302e1b94afbb49a0a26b6/tensorflow/compiler/tf2xla/kernels/conv_op_helpers.cc#L376
For the project I am working on, we mainly focus on inference, thus
RevOp(filter)can be further fold into a constantfilter', thus we don't needwindow_dilation.I am not sure if there are other use cases that we prefer tweaking the filter memory access pattern to a separate
RevOp.sdasgup3 commentedon Mar 8, 2023
Thanks @loganchien for the findings and confirming the current status. That's very useful!
sdasgup3 commentedon Mar 21, 2023
Update: We confirmed that none of the front-ends (TF/Jax/PyTorch), producing StableHLO, are using the
window_reversaland it could be safe to remove from StableHLO opsConvolutionOpandDynamicConvOp.I will further update the ticket with the process of removal.
sdasgup3 commentedon Mar 23, 2023
update: We now have an RFC to remove window_reversal from ConvolutionOp.
#1346