You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/01/05 09:27:50 UTC

[GitHub] [tvm] blackkker opened a new pull request #9840: Support -> Change the output shape calculation based on keep_dim option

blackkker opened a new pull request #9840:
URL: https://github.com/apache/tvm/pull/9840


   Support TODO in convert_fully_connected in tflite
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on a change in pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r784182743



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       yes




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] blackkker commented on a change in pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
blackkker commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r783663051



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       As the document said, I constructed the keras model with tf.keras.Input(shape=(1, 1, 768)) and tf.keras.layers.Dense(1001).
   ![code](https://user-images.githubusercontent.com/32191045/149277656-43fa4989-d665-4b7a-b21f-3e12cb02637b.png)
   And then i converted it to TFLite but got a fully connected op(keep_num_dims : False). 
   The specific implment in TFLite is to reshape into 2 dims before fullyconnect, and then reshape the output into 4dims.
   ![fullyconnected](https://user-images.githubusercontent.com/32191045/149277660-368e1efd-6c2f-454f-beb8-09874644f693.png)
   BTW, I tried tensorflow_v1 and tensorflow_v2, both failed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] blackkker commented on a change in pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
blackkker commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r783663051



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       As the document said, I constructed the keras model.
   ![code](https://user-images.githubusercontent.com/32191045/149277656-43fa4989-d665-4b7a-b21f-3e12cb02637b.png)
   And then i converted it to TFLite but got a fully connected op(keep_num_dims : False). 
   The specific implment in TFLite is to reshape into 2 dims before fullyconnect, and then reshape the output into 4dims.
   ![network](https://user-images.githubusercontent.com/32191045/149277873-aa29b472-323a-4332-b63a-a6e531a770ea.png)
   BTW, I tried tensorflow_v1 and tensorflow_v2, both failed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] blackkker commented on a change in pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
blackkker commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r784534510



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       Can 't fina a suitable line to add the `TODO` and suitable description about the todo.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] blackkker commented on a change in pull request #9840: Support -> Change the output shape calculation based on keep_dim option

Posted by GitBox <gi...@apache.org>.
blackkker commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r782674820



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       The fact is that I can not construct a fully_connected op with keep_dim == True or find a model which fully_connected op with keep_dim == True. 
   Also, I have tried to construct a matmul in tensorflow which can be converted to fullyconnected by tf.lite.TFLiteConverter. But, I just get the FlexMatmul op in TFLite.
   Now, i am stuck。
   BTW, I implement it according to [https://github.com/tensorflow/tensorflow/blob/master/tensorflow/lite/kernels/fully_connected.cc](url) line:353




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#issuecomment-1013487233


   > Hi~ there is a tflite optimization that may create a FC with `keep_num_dims=True` https://github.com/tensorflow/tensorflow/blob/31676d116a7ba7d07ac15c6ac03af09d903a6e6b/tensorflow/compiler/mlir/lite/transforms/optimize.cc#L598-L667
   
   That appears to rewrite an existing FC tflite with keep_num_dims. We need a way to get to construct an FC with that option in tflite (e.g. transform from regular TF)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo merged pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo merged pull request #9840:
URL: https://github.com/apache/tvm/pull/9840


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] wrongtest commented on pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
wrongtest commented on pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#issuecomment-1011866333


   Hi~ there is a tflite optimization that may create a FC with `keep_num_dims=True`
   https://github.com/tensorflow/tensorflow/blob/31676d116a7ba7d07ac15c6ac03af09d903a6e6b/tensorflow/compiler/mlir/lite/transforms/optimize.cc#L598-L667
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] blackkker commented on a change in pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
blackkker commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r783696945



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       Do you mean add a TODO: in `tests/python/frontend/tflite/test_forward.py` in the relevant spot.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on a change in pull request #9840: Support -> Change the output shape calculation based on keep_dim option

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r782471438



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       Can you add a test in `tests/python/frontend/tflite/test_forward.py`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on a change in pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r783664494



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       Ahh ok, can you add a test in `tests/python/frontend/tflite/test_forward.py` in the relevant spot saying we need to figure out how to make a tflite model with `keep_num_dims` and make a test. 
   
   I will probably accept PR since change is fairly simple.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] blackkker commented on a change in pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
blackkker commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r783663051



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       As the document said, I constructed the keras model.
   ![code](https://user-images.githubusercontent.com/32191045/149277656-43fa4989-d665-4b7a-b21f-3e12cb02637b.png)
   And then i converted it to TFLite but got a fully connected op(keep_num_dims : False). 
   The specific implment in TFLite is to reshape into 2 dims before fullyconnect, and then reshape the output into 4dims.
   ![fullyconnected](https://user-images.githubusercontent.com/32191045/149277660-368e1efd-6c2f-454f-beb8-09874644f693.png)
   BTW, I tried tensorflow_v1 and tensorflow_v2, both failed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] blackkker commented on a change in pull request #9840: Support -> Change the output shape calculation based on keep_dim option

Posted by GitBox <gi...@apache.org>.
blackkker commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r782674820



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       The fact is that I can not new a fully_connected op with keep_dim == True or find a model which fully_connected op with keep_dim == True. 
   Also, I have tried to construct a matmul in tensorflow which can be converted to fullyconnected by tf.lite.TFLiteConverter. But, I just get the FlexMatmul op in TFLite.
   Now, i am stuck。
   BTW, I implement it according to [https://github.com/tensorflow/tensorflow/blob/master/tensorflow/lite/kernels/fully_connected.cc](url) line:353




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on a change in pull request #9840: [Relay/Frontend][TFLite] Change the output shape calculation based on keep_dim option in fully connected

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #9840:
URL: https://github.com/apache/tvm/pull/9840#discussion_r783397015



##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -1911,6 +1911,14 @@ def convert_fully_connected(self, op):
             weight_expr = self.exp_tab.new_const(weight_value, dtype=weight_tensor_type_str)
         weight_shape = _infer_shape(weight_expr)
 
+        # Change the output shape calculation based on keep_dim option

Review comment:
       Maybe try converting from keras?
   
   https://www.tensorflow.org/api_docs/python/tf/keras/layers/Dense
   
   Its documentated behavior seems to match keep_dims = True.
   
   I would make just a new unit test if you go this route.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org