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/07 07:54:33 UTC

[GitHub] [tvm] mshr-h commented on a change in pull request #9655: [Caffe Frontend] Add support for Power layer

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: commits-unsubscribe@tvm.apache.org

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