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/11/08 17:39:28 UTC

[GitHub] [tvm] srkreddy1238 opened a new pull request, #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

srkreddy1238 opened a new pull request, #13325:
URL: https://github.com/apache/tvm/pull/13325

   Heterogeneous module partitioned by BYOC has functions nodes without any VirtualDevice definition (having FullyUnconstrained device). Ignoring the device here causes expr_virtual_devices_ being empty when PopVirtualDevice is called assuming above PushVirtualDevice is succeeded. PushVirtualDevice and PopVirtualDevice occurs as pairs across function body, hence it's better to insert the The Virtual Device for Uncontrained and Pop it subsequently.


-- 
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] srkreddy1238 commented on pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on PR #13325:
URL: https://github.com/apache/tvm/pull/13325#issuecomment-1330818742

   @junrushao can you take a look on this ?


-- 
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] elvin-n commented on pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
elvin-n commented on PR #13325:
URL: https://github.com/apache/tvm/pull/13325#issuecomment-1332173716

   > cc @echuraev @elvin-n does this change look good to you and pass internal texture 
   
   Our tests runs in CI and if we see green CI it means this change does not affect them and whole flow. I cannot answer if this change affects origin Mark's idea and if it does not brake anything in other places. Again, assuming that we have many tests for VirtualDevices and they passed, this change should be safe.


-- 
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] csullivan commented on pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
csullivan commented on PR #13325:
URL: https://github.com/apache/tvm/pull/13325#issuecomment-1307976117

   Hi @srkreddy1238, I didn't quite understand the conclusion you make,  
   
   > PushVirtualDevice and PopVirtualDevice occurs as pairs across function body, **hence it's better to insert the The Virtual Device for Uncontrained and Pop it subsequently**
   
   Naively I can understand why we skip pushing an unconstrained virtual device: If something is fully unconstrained then it's virtual device shouldn't influence the function body visitation. What issue is this causing in BYOC? 
   
   Is there a unit test you can consider contributing that can help illustrate the issue?


-- 
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] junrushao commented on pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
junrushao commented on PR #13325:
URL: https://github.com/apache/tvm/pull/13325#issuecomment-1357117905

   Sure! It’s been approved for a while already. Not sure why it’s not merged yet


-- 
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] srkreddy1238 commented on pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on PR #13325:
URL: https://github.com/apache/tvm/pull/13325#issuecomment-1315244355

   @mbs-octoml 


-- 
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] tvm-bot commented on pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13325:
URL: https://github.com/apache/tvm/pull/13325#issuecomment-1307593897

   <!---bot-comment-->
   
   Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
   
   <!--bot-comment-ccs-start-->
    * No users to tag found in teams: `transform` <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
   
   <sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>


-- 
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] srkreddy1238 commented on pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on PR #13325:
URL: https://github.com/apache/tvm/pull/13325#issuecomment-1308171709

   
   PushVirtualDevice and PopVirtualDevice maintains the current VirtualDevice information on stack ```expr_virtual_devices_ ```. These calls are invoked across a block parsing (FunctionNodes, CallNode ..etc). The stack (expr_virtual_devices_) grows as we nest into the functions.
   
   ```
   def @tvmgen_default_clml_main_17(%clml_17_i0: Tensor[(1, 2048), float32] /* ty=Tensor[(1, 2048), float32] */, Inline=1, Compiler="clml", global_symbol="tvmgen_default_clml_main_17", Primitive=1) -> Tensor[(1, 1000), float32] {
     %245 = fn (%FunctionVar_0_01: Tensor[(1, 2048), float32] /* ty=Tensor[(1, 2048), float32] */, PartitionedFromPattern="nn.dense_nn.bias_add_", Composite="clml.dense") -> Tensor[(1, 1000), float32] {
       %244 = nn.dense(%FunctionVar_0_01, meta[relay.Constant][257] /* ty=Tensor[(1000, 2048), float32] */, units=1000) /* ty=Tensor[(1, 1000), float32] */;
       add(%244, meta[relay.Constant][258] /* ty=Tensor[(1, 1000), float32] */) /* ty=Tensor[(1, 1000), float32] */
     } /* ty=fn (Tensor[(1, 2048), float32]) -> Tensor[(1, 1000), float32] */;
     %245(%clml_17_i0) /* ty=Tensor[(1, 1000), float32] */
   }
   
   ```
   
   In the above IR the composite function ```%245 = fn (%FunctionVar_0_01``` will not have a VirtualDevice definition (``` VirtualDevice(?)```) and the PushVirtualDevice fails. Whereas at end of this function parsing the PopVirtualDevice removes the VirtualDevice corresponding to the our scope function. From here all annotations fail due to empty stack.
   
   
   I figured it out as part of large network. I am working on to facilitate a test case for the same.
    


-- 
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] junrushao merged pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
junrushao merged PR #13325:
URL: https://github.com/apache/tvm/pull/13325


-- 
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] srkreddy1238 commented on pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on PR #13325:
URL: https://github.com/apache/tvm/pull/13325#issuecomment-1357094908

   @junrushao can you conclude this PR ?  There are some follow up on this.


-- 
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] csullivan commented on pull request #13325: [TRANSFORM] Fix virtual device annotation issue with BYOC subgraphs

Posted by GitBox <gi...@apache.org>.
csullivan commented on PR #13325:
URL: https://github.com/apache/tvm/pull/13325#issuecomment-1331015785

   cc @echuraev @elvin-n does this change look good to you and pass internal texture tests?


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