gemini-code-assist[bot] commented on code in PR #19523:
URL: https://github.com/apache/tvm/pull/19523#discussion_r3211131961


##########
python/tvm/relax/frontend/tflite/tflite_frontend.py:
##########
@@ -2449,6 +2450,143 @@ def convert_conv(self, op, conv_type):
             out = self.convert_fused_activation_function(out, 
fused_activation_fn)
         return out
 
+    def convert_conv3d(self, op):
+        """3D convolution implementation."""
+
+        from tflite.BuiltinOptions import BuiltinOptions
+        from tflite.Conv3DOptions import Conv3DOptions
+        from tflite.Padding import Padding
+        from tflite.TensorType import TensorType
+
+        input_tensors = self.get_input_tensors(op)
+        assert len(input_tensors) >= 2, "input tensors length should be >= 2"
+        
+        input_tensor = input_tensors[0]
+        input_tensor_idx = input_tensor.tensor_idx
+        weight_tensor = input_tensors[1]
+
+        output_tensors = self.get_output_tensors(op)
+        assert len(output_tensors) == 1, "output tensors length should be 1"
+        output_tensor = output_tensors[0]
+
+        assert op.BuiltinOptionsType() == BuiltinOptions.Conv3DOptions
+        op_options = op.BuiltinOptions()
+        conv3d_options = Conv3DOptions()
+        conv3d_options.Init(op_options.Bytes, op_options.Pos)
+
+        stride_d = conv3d_options.StrideD()
+        stride_h = conv3d_options.StrideH()
+        stride_w = conv3d_options.StrideW()
+        dilation_d = conv3d_options.DilationDFactor()
+        dilation_h = conv3d_options.DilationHFactor()
+        dilation_w = conv3d_options.DilationWFactor()
+        padding = conv3d_options.Padding()
+        fused_activation_fn = conv3d_options.FusedActivationFunction()
+
+        _, input_d, input_h, input_w, input_c = 
to_int_list(self.get_tensor_shape(input_tensor))
+        # TFLite Conv3D kernel layout is already DHWIO:
+        # KD KH KW IC OC
+        kernel_d, kernel_h, kernel_w, in_channels, output_channels = 
to_int_list(
+            self.get_tensor_shape(weight_tensor)
+        )
+
+        dilated_kernel_d = dilation_d * (kernel_d - 1) + 1
+        dilated_kernel_h = dilation_h * (kernel_h - 1) + 1
+        dilated_kernel_w = dilation_w * (kernel_w - 1) + 1
+
+        params = {
+            # "kernel_size": [kernel_d, kernel_h, kernel_w],
+            "strides": [stride_d, stride_h, stride_w],
+            "dilation": [dilation_d, dilation_h, dilation_w],
+            "padding": [0, 0, 0],
+            "data_layout": "NDHWC",
+        }
+
+        params["kernel_layout"] = "DHWIO"
+        if input_c != in_channels:
+            assert input_c % in_channels == 0, (
+                "Input channels is not divisible of kernel in_channels."

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   Typo in the error message: "divisible of" should be "divisible by".
   
   ```suggestion
                   "Input channels is not divisible by kernel in_channels."
   ```



##########
python/tvm/relax/frontend/tflite/tflite_frontend.py:
##########
@@ -2449,6 +2450,143 @@ def convert_conv(self, op, conv_type):
             out = self.convert_fused_activation_function(out, 
fused_activation_fn)
         return out
 
+    def convert_conv3d(self, op):
+        """3D convolution implementation."""
+
+        from tflite.BuiltinOptions import BuiltinOptions
+        from tflite.Conv3DOptions import Conv3DOptions
+        from tflite.Padding import Padding
+        from tflite.TensorType import TensorType
+
+        input_tensors = self.get_input_tensors(op)
+        assert len(input_tensors) >= 2, "input tensors length should be >= 2"
+        
+        input_tensor = input_tensors[0]
+        input_tensor_idx = input_tensor.tensor_idx
+        weight_tensor = input_tensors[1]
+
+        output_tensors = self.get_output_tensors(op)
+        assert len(output_tensors) == 1, "output tensors length should be 1"
+        output_tensor = output_tensors[0]
+
+        assert op.BuiltinOptionsType() == BuiltinOptions.Conv3DOptions
+        op_options = op.BuiltinOptions()
+        conv3d_options = Conv3DOptions()
+        conv3d_options.Init(op_options.Bytes, op_options.Pos)
+
+        stride_d = conv3d_options.StrideD()
+        stride_h = conv3d_options.StrideH()
+        stride_w = conv3d_options.StrideW()
+        dilation_d = conv3d_options.DilationDFactor()
+        dilation_h = conv3d_options.DilationHFactor()
+        dilation_w = conv3d_options.DilationWFactor()
+        padding = conv3d_options.Padding()
+        fused_activation_fn = conv3d_options.FusedActivationFunction()
+
+        _, input_d, input_h, input_w, input_c = 
to_int_list(self.get_tensor_shape(input_tensor))
+        # TFLite Conv3D kernel layout is already DHWIO:
+        # KD KH KW IC OC
+        kernel_d, kernel_h, kernel_w, in_channels, output_channels = 
to_int_list(
+            self.get_tensor_shape(weight_tensor)
+        )
+
+        dilated_kernel_d = dilation_d * (kernel_d - 1) + 1
+        dilated_kernel_h = dilation_h * (kernel_h - 1) + 1
+        dilated_kernel_w = dilation_w * (kernel_w - 1) + 1
+
+        params = {
+            # "kernel_size": [kernel_d, kernel_h, kernel_w],
+            "strides": [stride_d, stride_h, stride_w],

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   Remove the commented-out code to keep the implementation clean and 
maintainable.
   
   ```suggestion
               "strides": [stride_d, stride_h, stride_w],
   ```



##########
python/tvm/relax/frontend/tflite/tflite_frontend.py:
##########
@@ -2449,6 +2450,143 @@ def convert_conv(self, op, conv_type):
             out = self.convert_fused_activation_function(out, 
fused_activation_fn)
         return out
 
+    def convert_conv3d(self, op):
+        """3D convolution implementation."""
+
+        from tflite.BuiltinOptions import BuiltinOptions
+        from tflite.Conv3DOptions import Conv3DOptions
+        from tflite.Padding import Padding
+        from tflite.TensorType import TensorType
+
+        input_tensors = self.get_input_tensors(op)
+        assert len(input_tensors) >= 2, "input tensors length should be >= 2"
+        
+        input_tensor = input_tensors[0]
+        input_tensor_idx = input_tensor.tensor_idx
+        weight_tensor = input_tensors[1]
+
+        output_tensors = self.get_output_tensors(op)
+        assert len(output_tensors) == 1, "output tensors length should be 1"
+        output_tensor = output_tensors[0]
+
+        assert op.BuiltinOptionsType() == BuiltinOptions.Conv3DOptions
+        op_options = op.BuiltinOptions()
+        conv3d_options = Conv3DOptions()
+        conv3d_options.Init(op_options.Bytes, op_options.Pos)
+
+        stride_d = conv3d_options.StrideD()
+        stride_h = conv3d_options.StrideH()
+        stride_w = conv3d_options.StrideW()
+        dilation_d = conv3d_options.DilationDFactor()
+        dilation_h = conv3d_options.DilationHFactor()
+        dilation_w = conv3d_options.DilationWFactor()
+        padding = conv3d_options.Padding()
+        fused_activation_fn = conv3d_options.FusedActivationFunction()
+
+        _, input_d, input_h, input_w, input_c = 
to_int_list(self.get_tensor_shape(input_tensor))
+        # TFLite Conv3D kernel layout is already DHWIO:
+        # KD KH KW IC OC
+        kernel_d, kernel_h, kernel_w, in_channels, output_channels = 
to_int_list(
+            self.get_tensor_shape(weight_tensor)
+        )
+
+        dilated_kernel_d = dilation_d * (kernel_d - 1) + 1
+        dilated_kernel_h = dilation_h * (kernel_h - 1) + 1
+        dilated_kernel_w = dilation_w * (kernel_w - 1) + 1
+
+        params = {
+            # "kernel_size": [kernel_d, kernel_h, kernel_w],
+            "strides": [stride_d, stride_h, stride_w],
+            "dilation": [dilation_d, dilation_h, dilation_w],
+            "padding": [0, 0, 0],

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   For 3D convolution, it is more consistent to use a 6-element padding list 
(front, top, left, back, bottom, right) even for the zero-padding case. This 
aligns with the 6-element format used in the `SAME` padding logic and matches 
the expected IR structure in the unit tests.
   
   ```suggestion
               "padding": [0, 0, 0, 0, 0, 0],
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to