One: Remove redundant TRANSPOSE Ops generated by onnx-tf

Created on 17 Dec 2020  路  11Comments  路  Source: Samsung/ONE

What

Remove redundant TRANSPOSE Ops generated by onnx-tf

Why

We're going to support ONNX model in ONE via the following steps.

ONNX -> TF (.pb) -> TFLite (.tflite) -> Circle (.circle)

onnx-tensorflow will be used to convert ONNX file to TF (.pb) file. It generates many Transpose Ops to convert the data layout from NCHW (used by ONNX) to NHWC (used by TF). These Transpose Ops are indeed meaningless in terms of DNN behavior, but introduce a huge overhead.

How

To remove these Transpose Ops, we'll change NCHW Ops to NHWC Ops and use RemoveRedundantTransposePass. For example,

Before: NCHW tensor -> Op (NCHW) -> NCHW tensor

After: NCHW tensor -> Transpose (perm:[0,2,3,1]) -> NHWC tensor -> Op (NHWC) -> NHWC tensor -> Transpose (perm:[0,3,1,2]) -> NCHW tensor

onnx-tensorflow does this transformation only for special Ops (e.g., Conv, TConv, etc). We're going to do this transformation for all Ops to make RemoveRedundantTransposePass remove redundant Transpose Ops.

For example, let's assume the above transformation is done for two subsequent Ops

NCHW tensor -> Transpose (perm:[0,2,3,1]) -> NHWC tensor -> Op (changed to use NHWC) -> NHWC tensor -> Transpose (perm:[0,3,1,2]) -> NCHW tensor -> Transpose (perm:[0,2,3,1]) -> NHWC tensor -> Op (changed to use NHWC) -> NHWC tensor -> Transpose (perm:[0,3,1,2]) -> NCHW tensor

The bold Transpose Ops can be removed by RemoveRedundantTransposePass as they are redundant.

For this, we need a pass (ConvertNCHWToNHWCPass) that updates NCHW Ops to NHWC Ops.

Status

  • [x] Convert NCHW to NHWC #5438 @jinevening
  • [x] Remove redundant transpose #5437 @struss
  • [x] Remove redundant split #5456 @struss
  • [x] Remove redundant slice #5416 @struss
  • [x] Convert Transpose to Reshape #5703 @struss
  • [x] Remove redundant Reshape #5275 @struss

Most helpful comment

TODO. Improve RemoveRedundantTransposePass to cover the below case. @struss Can you help with this?

Before

                      |
                  Transpose (0,3,1,2)
                   /     \
Transpose (0,2,3,1)    Transpose (0,2,3,1)
  |                       |
 Op2                      Op3

After

                   /     \
                Op2       Op3

@jinevening Okay I will update RemoveRedundantTransposePass to cover this case.

All 11 comments

I'm working on draft. MUL is transformed as below.

image

ADD and PAD are transformed similarly.

Transpose Ops between the converted Ops can be removed by RemoveRedundantTransposePass. The graph applied with RemoveRedundantTransposePass is shown below.

image

TODO. Improve RemoveRedundantTransposePass to cover the below case. @struss Can you help with this?

Before

                      |
                  Transpose (0,3,1,2)
                   /     \
Transpose (0,2,3,1)    Transpose (0,2,3,1)
  |                       |
 Op2                      Op3

After

                   /     \
                Op2       Op3

TODO. Improve RemoveRedundantTransposePass to cover the below case. @struss Can you help with this?

Before

                      |
                  Transpose (0,3,1,2)
                   /     \
Transpose (0,2,3,1)    Transpose (0,2,3,1)
  |                       |
 Op2                      Op3

After

                   /     \
                Op2       Op3

@jinevening Okay I will update RemoveRedundantTransposePass to cover this case.

Current logic only handle this case below

   |
Transpose 
   | (with only one passing)
 Transpose  
   | 
(any number of succs)

to change to reuse first node below.

   |
  Transpose (with updated CircleConst Perm)
   |       \
   |        \
 Transpose   |
             |
(any number of succs)

I think change logic to create new transpose.

Another issue: Input and Output

onnx-tf inserts Transpose Ops at the beginning and at the end of the model to convert NCHW to NHWC.

image

ConvertNCHWToNHWCPass will change the input shape to NHWC and remove the Transpose Ops.

Another issue.

onnx-tf inserts RESHAPE instead of TRANSPOSE if possible.

image

Reshape->Transpose in the graph is an identity function, so we can remove them. But currently this pattern is not captured by RemoveRedundantTransposePass.

@struss Can you extend RemoveRedundantTransposePass to cover the above case as well (after #5437 is merged) ?

I think it would be better to handle the case in RemoveRedundantTransposePass rather than making another pass.

@struss Can you extend RemoveRedundantTransposePass to cover the above case as well (after #5437 is merged) ?

Okay I will do this.

I think it would be better to handle the case in RemoveRedundantTransposePass rather than making another pass.

then I think I need to rename RemoveRedundantTransposePass to something new.

@jinevening I think we need to consider reverse order case also to be handled.

CircleNode -> CircleTranspose -> CircleReshape -> CircleNode

I think it will also need to be handled.
so, what about adding a pass for Transpose to Reshape or Reshape to Transpose?

Reshape is cheaper than Transpose in general, so it would be better to transform Transpose to Reshape and remove redundant Reshape Ops (You've already made a draft! #5266).

All tasks seems to be finished. Can we close this issue? or maybe add more tasks?

All tasks seems to be finished. Can we close this issue? or maybe add more tasks?

Let's close this issue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

periannath picture periannath  路  3Comments

mhs4670go picture mhs4670go  路  4Comments

ragmani picture ragmani  路  4Comments

periannath picture periannath  路  3Comments

mhs4670go picture mhs4670go  路  3Comments