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 2020/04/20 14:42:42 UTC

[GitHub] [incubator-tvm] siju-samuel opened a new pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

siju-samuel opened a new pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383


   Pytorch frontend op support
   - where
   - addcdiv
   - addcmul
   
   @masahi please help to review this pytorch ops. Thanks.


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

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



[GitHub] [incubator-tvm] masahi commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412081790



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):

Review comment:
       ```
       if isinstance(inp, (_expr.Var, _expr.Call):
           data = inp
   
   ```




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

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



[GitHub] [incubator-tvm] masahi commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412081790



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):

Review comment:
       ```
       if isinstance(inp, (_expr.Var, _expr.Call)):
           data = inp
   
   ```




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

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



[GitHub] [incubator-tvm] masahi commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412082233



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):
+        data = inp
+    elif isinstance(inp, _expr.Call):
+        data = inp
+    elif isinstance(inp, torch.Tensor):

Review comment:
       Does this really happen? I don't expect that. Could be a logic bug if it does.
   If this doesn't happen, I don't think we need this function.




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

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



[GitHub] [incubator-tvm] masahi commented on pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#issuecomment-618941150


   Thanks @siju-samuel 


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

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



[GitHub] [incubator-tvm] siju-samuel commented on pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
siju-samuel commented on pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#issuecomment-618937603


   @masahi  Could you please review and merge. Thanks.


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

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



[GitHub] [incubator-tvm] masahi commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412082233



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):
+        data = inp
+    elif isinstance(inp, _expr.Call):
+        data = inp
+    elif isinstance(inp, torch.Tensor):

Review comment:
       Does this really happen? I don't expect that.




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

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



[GitHub] [incubator-tvm] siju-samuel commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412128043



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):
+        data = inp
+    elif isinstance(inp, _expr.Call):
+        data = inp
+    elif isinstance(inp, torch.Tensor):

Review comment:
       ok. i will fix this and do the change accordingly. thanks.




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

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



[GitHub] [incubator-tvm] siju-samuel commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412097547



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):
+        data = inp
+    elif isinstance(inp, _expr.Call):
+        data = inp
+    elif isinstance(inp, torch.Tensor):

Review comment:
       ![image](https://user-images.githubusercontent.com/15828974/79860349-25ee8600-83f0-11ea-8552-3e4c04a1bf97.png)
   
   You can see the type of inputs, which im printing.




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

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



[GitHub] [incubator-tvm] siju-samuel commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412107805



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):
+        data = inp
+    elif isinstance(inp, _expr.Call):
+        data = inp
+    elif isinstance(inp, torch.Tensor):

Review comment:
       ![image](https://user-images.githubusercontent.com/15828974/79861643-5800e780-83f2-11ea-8bff-f9d76382d2c3.png)
   
   Actually i never saw numpy array as input, see above image, here the second input is `tensor([[0, 0],
           [1, 0]])` which is torch.Tensor




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

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



[GitHub] [incubator-tvm] masahi commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412113613



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):
+        data = inp
+    elif isinstance(inp, _expr.Call):
+        data = inp
+    elif isinstance(inp, torch.Tensor):

Review comment:
       ok this is because of this line
   https://github.com/apache/incubator-tvm/blob/22db299b33f05570db2a5a406bdb37b57198a822/python/tvm/relay/frontend/pytorch.py#L1832
   
   Can you apply `_wrap_const` on it? See how badly it breaks the rest of test cases. Returning a raw torch tensor is not a good idea anyway.




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

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



[GitHub] [incubator-tvm] masahi commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412082233



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):
+        data = inp
+    elif isinstance(inp, _expr.Call):
+        data = inp
+    elif isinstance(inp, torch.Tensor):

Review comment:
       Does this really happen? I don't expect that. Could be a logic bug if it does.




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

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



[GitHub] [incubator-tvm] siju-samuel commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412094229



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):

Review comment:
       if one of the input is `torch.ones,`  it will first convert to `_expr.Call `for `torch.ones`, for these kind of inputs it will happen. actually we only need to differentiate `torch.Tensor` as tvm ops cannot input `torch.tensor`. 
   Any tvm expr as input, tvm will handle. 
   Whereever having check for `_expr.var` & `torch.tensor` is there, need to handle `_expr.call` also.




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

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



[GitHub] [incubator-tvm] masahi commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412099376



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):

Review comment:
       I mean the body of Var and Call case is the same, `data = inp`, you don't need two if blocks




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

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



[GitHub] [incubator-tvm] siju-samuel commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412096961



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):

Review comment:
       ![image](https://user-images.githubusercontent.com/15828974/79860322-153e1000-83f0-11ea-982f-e0f4b561a23b.png)
   




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

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



[GitHub] [incubator-tvm] masahi commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412101164



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):
+        data = inp
+    elif isinstance(inp, _expr.Call):
+        data = inp
+    elif isinstance(inp, torch.Tensor):

Review comment:
       I still don't see how we get torch.Tensor as input? Inputs should always be relay value. From Torch tensor, we should get the numpy array, and wrap relay.const on it. Having torch tensor in op inputs is a logic bug.  




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

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



[GitHub] [incubator-tvm] siju-samuel commented on a change in pull request #5383: [PYTORCH]where, addcdiv, addcmul op support

Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #5383:
URL: https://github.com/apache/incubator-tvm/pull/5383#discussion_r412096961



##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -337,6 +337,55 @@ def _impl(inputs, input_types):
         return _op.transform.repeat(data, repeats=repeats, axis=axis)
     return _impl
 
+
+def _parse_input_data(inp):
+    import torch
+    if isinstance(inp, _expr.Var):

Review comment:
       ![image](https://user-images.githubusercontent.com/15828974/79860322-153e1000-83f0-11ea-982f-e0f4b561a23b.png)
   




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

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