mshr-h commented on a change in pull request #9655:
URL: https://github.com/apache/tvm/pull/9655#discussion_r780078251



##########
File path: python/tvm/relay/frontend/caffe.py
##########
@@ -637,6 +638,22 @@ def convert_embed(self, op):
 
         return out
 
+    def convert_power(self, op):
+        """Convert Power layer"""
+        inputs = op.bottom
+        in_expr = self.exp_tab.get_expr(inputs[0])
+        power = _expr.const(op.power_param.power)
+        scale = _expr.const(op.power_param.scale)
+        shift = _expr.const(op.power_param.shift)
+
+        if scale != 1.0:
+            out = _op.multiply(in_expr, scale)
+        if shift != 0.0:
+            out = _op.add(out, shift)
+        if power != 1.0:
+            out = _op.power(out, power)

Review comment:
       Same here.

##########
File path: tests/python/frontend/caffe/test_forward.py
##########
@@ -613,6 +613,24 @@ def test_forward_Pooling():
     _test_pooling(data, pool=P.Pooling.AVE, global_pooling=True)
 
 
+#######################################################################
+# Power
+# -----------
+def _test_power(data, **kwargs):
+    """One iteration of Power."""
+    _test_op(data, L.Power, "Power", **kwargs)
+
+
+def test_forward_Power():
+    """Power"""
+    data = np.random.rand(1, 3, 10, 10).astype(np.float32)
+    _test_power(data, power_param={"power": 0.37, "scale": 0.83, "shift": 
-2.4})
+    _test_power(data, power_param={"power": 0.37, "scale": 0.83, "shift": 0.0})
+    _test_power(data, power_param={"power": 0.0, "scale": 0.83, "shift": -2.4})
+    _test_power(data, power_param={"power": 1.0, "scale": 0.83, "shift": -2.4})
+    _test_power(data, power_param={"power": 2.0, "scale": 0.34, "shift": -2.4})
+

Review comment:
       Might be better to add a test with Caffe's default parameters.
   
   ```python
   _test_power(data, power_param={"power": 1.0, "scale": 1.0, "shift": 0.0})
   ```
   

##########
File path: python/tvm/relay/frontend/caffe.py
##########
@@ -637,6 +638,22 @@ def convert_embed(self, op):
 
         return out
 
+    def convert_power(self, op):
+        """Convert Power layer"""
+        inputs = op.bottom
+        in_expr = self.exp_tab.get_expr(inputs[0])
+        power = _expr.const(op.power_param.power)
+        scale = _expr.const(op.power_param.scale)
+        shift = _expr.const(op.power_param.shift)
+
+        if scale != 1.0:
+            out = _op.multiply(in_expr, scale)
+        if shift != 0.0:
+            out = _op.add(out, shift)
+        if power != 1.0:
+            out = _op.power(out, power)
+        return out

Review comment:
       I think we don't need to check the parameters.
   Just like below.
   
   ```python
   out = _op.multiply(in_expr, scale)
   out = _op.add(out, shift)
   out = _op.power(out, power)
   return out
   ```

##########
File path: python/tvm/relay/frontend/caffe.py
##########
@@ -637,6 +638,22 @@ def convert_embed(self, op):
 
         return out
 
+    def convert_power(self, op):
+        """Convert Power layer"""
+        inputs = op.bottom
+        in_expr = self.exp_tab.get_expr(inputs[0])
+        power = _expr.const(op.power_param.power)
+        scale = _expr.const(op.power_param.scale)
+        shift = _expr.const(op.power_param.shift)
+
+        if scale != 1.0:
+            out = _op.multiply(in_expr, scale)
+        if shift != 0.0:
+            out = _op.add(out, shift)

Review comment:
       if `scale` is 1.0, line 650 won't be executed.
   Then `out` is undefined which causes an undefined reference error. 

##########
File path: tests/python/frontend/caffe/test_forward.py
##########
@@ -613,6 +613,24 @@ def test_forward_Pooling():
     _test_pooling(data, pool=P.Pooling.AVE, global_pooling=True)
 
 
+#######################################################################
+# Power
+# -----------

Review comment:
       Can you remove unnecessary redundant `--` and keep aligned with Power?
   
   ```python
   #######################################################################
   # Power
   # -----
   ```

##########
File path: python/tvm/relay/frontend/caffe.py
##########
@@ -637,6 +638,22 @@ def convert_embed(self, op):
 
         return out
 
+    def convert_power(self, op):
+        """Convert Power layer"""
+        inputs = op.bottom
+        in_expr = self.exp_tab.get_expr(inputs[0])
+        power = _expr.const(op.power_param.power)
+        scale = _expr.const(op.power_param.scale)
+        shift = _expr.const(op.power_param.shift)
+
+        if scale != 1.0:
+            out = _op.multiply(in_expr, scale)
+        if shift != 0.0:
+            out = _op.add(out, shift)
+        if power != 1.0:
+            out = _op.power(out, power)
+        return out

Review comment:
       If `scale` is 1.0 and `shift` is 0.0 and `power` is 1.0, `out` will be 
undefined.




-- 
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]


Reply via email to