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/03/22 21:28:22 UTC

[GitHub] [tvm] Lunderberg opened a new pull request #10712: [NDArray] Expose NDArray::CreateView to python

Lunderberg opened a new pull request #10712:
URL: https://github.com/apache/tvm/pull/10712


   Modifying the array view is needed for Hexagon targets, in order to first call `tvm.nd.array` with the physical dimensions, then update the shape to contain the logical dimensions.


-- 
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] tqchen commented on a change in pull request #10712: [NDArray] Expose NDArray::CreateView to python

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



##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -249,6 +249,28 @@ def copyto(self, target):
             return self._copyto(res)
         raise ValueError("Unsupported target type %s" % str(type(target)))
 
+    def create_view(self, shape):

Review comment:
       Let us make it _create_view for now. 
   
   Using views extensively can be unsafe, and should be discouraged. Note that most of TVM function compiled assumes no-alias, which works well when views are not created in frontend.
   
   The particular application of logical/physical layout is a good one that can use the low-level API. but would also be good to think how to make the API safe in the first place(e.g. when specify the physical shape we "consume" the original tensor so it will no longer be used).
   
   Mark it as private with note would help to make this issue clear. 




-- 
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] Lunderberg commented on a change in pull request #10712: [NDArray] Expose NDArray::CreateView to python

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



##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -249,6 +249,20 @@ def copyto(self, target):
             return self._copyto(res)
         raise ValueError("Unsupported target type %s" % str(type(target)))
 
+    def create_view(self, shape):
+        shape_imm = []

Review comment:
       (Related PR #10728 made to update the `TVMArrayAllocWithScope` signature to also use `ShapeTuple`, as long as I'm thinking on it.)




-- 
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] Lunderberg commented on a change in pull request #10712: [NDArray] Expose NDArray::CreateView to python

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



##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -249,6 +249,28 @@ def copyto(self, target):
             return self._copyto(res)
         raise ValueError("Unsupported target type %s" % str(type(target)))
 
+    def create_view(self, shape):

Review comment:
       Agreed, and updated the name to `_create_view` and the docstring to include a warning.
   
   In the future, I want to make it easier to interact with the different shapes (logical shape, transformed shape, physical shape), but am still working through the different interaction points needed.  With respect to a safe API here, I *think* that would either be `tvm.nd.empty` allow the tensor shape and the shape of the backing allocation to be independently specified, but I haven't finished fleshing out that design.




-- 
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] junrushao1994 merged pull request #10712: [NDArray] Expose NDArray::CreateView to python

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


   


-- 
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] Lunderberg commented on a change in pull request #10712: [NDArray] Expose NDArray::CreateView to python

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



##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -249,6 +249,20 @@ def copyto(self, target):
             return self._copyto(res)
         raise ValueError("Unsupported target type %s" % str(type(target)))
 
+    def create_view(self, shape):
+        shape_imm = []

Review comment:
       Thank you, and that cleans it up quite a bit.  I had been using the same calling convention as `TVMArrayAllocWithScope`, called from `tvm.runtime.ndarray.empty`, which predates `ShapeTuple`.




-- 
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] junrushao1994 commented on a change in pull request #10712: [NDArray] Expose NDArray::CreateView to python

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



##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -249,6 +249,20 @@ def copyto(self, target):
             return self._copyto(res)
         raise ValueError("Unsupported target type %s" % str(type(target)))
 
+    def create_view(self, shape):
+        shape_imm = []

Review comment:
       nit: shall we just ShapeTuple instead of raw pointer to np array?




-- 
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] tqchen commented on a change in pull request #10712: [NDArray] Expose NDArray::CreateView to python

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



##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -249,6 +249,28 @@ def copyto(self, target):
             return self._copyto(res)
         raise ValueError("Unsupported target type %s" % str(type(target)))
 
+    def create_view(self, shape):

Review comment:
       Consider make it _create_view for now. 
   
   Using views extensively can be unsafe, and should be discouraged. Note that most of TVM function compiled assumes no-alias, which works well when views are not created in frontend. No-alias is generally needed for performance. 
   
   The lower-level can choose to create views, but still with care that the views do not alias in each call
   
   The particular application of logical/physical layout is a good one that can use the low-level API. but would also be good to think how to make the API safe in the first place(e.g. when specify the physical shape we "consume" the original tensor so it will no longer be used).
   
   Mark it as private with note would help to make this issue clear. 




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