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/28 11:37:23 UTC

[GitHub] [tvm] anilmartha opened a new pull request #10802: Ci fix

anilmartha opened a new pull request #10802:
URL: https://github.com/apache/tvm/pull/10802


   Hi @leandron,
   
   We have fixed the vitis-ai test failure [issue ](https://github.com/apache/tvm/issues/10757)when running with USE_VITIS_AI OFF flag. Could you please review and merge 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] manupa-arm commented on pull request #10802: [CI] Fix Vitis-AI tests when USE_VITIS_AI flag set to OFF

Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #10802:
URL: https://github.com/apache/tvm/pull/10802#issuecomment-1081973787


   Thanks @anilmartha! LGTM!


-- 
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] ashutosh-arm commented on a change in pull request #10802: [CI] Fix Vitis-AI tests when USE_VITIS_AI flag set to OFF

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on a change in pull request #10802:
URL: https://github.com/apache/tvm/pull/10802#discussion_r836452538



##########
File path: tests/python/contrib/test_vitis_ai/infrastructure.py
##########
@@ -59,7 +59,7 @@ def visit_call(self, call):
 
 def skip_test():

Review comment:
       Overall LGTM! I see that tvm has more or less a standard way of defining require_*() functions in this file and using those functions to skip the tests. Maybe you could define a similar function for vitis_ai.
   https://github.com/apache/tvm/blob/main/python/tvm/testing/utils.py




-- 
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] manupa-arm commented on pull request #10802: [CI] Fix Vitis-AI tests when USE_VITIS_AI flag set to OFF

Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #10802:
URL: https://github.com/apache/tvm/pull/10802#issuecomment-1082249635


   Thanks @anilmartha @ashutosh-arm @driazati !


-- 
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] anilmartha commented on a change in pull request #10802: [CI] Fix Vitis-AI tests when USE_VITIS_AI flag set to OFF

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



##########
File path: tests/python/contrib/test_vitis_ai/test_vitis_ai_codegen.py
##########
@@ -54,6 +54,8 @@ def set_func_attr(func, compile_name, symbol_name):
 )
 def test_conv2d(dpu_target):

Review comment:
       @driazati 
   I replaced skip_test with pytest's skipif. Could you have a another look?




-- 
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] anilmartha commented on pull request #10802: [CI] Fix Vitis-AI tests when USE_VITIS_AI flag set to OFF

Posted by GitBox <gi...@apache.org>.
anilmartha commented on pull request #10802:
URL: https://github.com/apache/tvm/pull/10802#issuecomment-1081970289


   @manupa-arm @ashutosh-arm 
   Now,  I have added requires_vitis_ai function to consistent with other codegens. 


-- 
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] anilmartha commented on pull request #10802: [CI] Fix Vitis-AI tests when USE_VITIS_AI flag set to OFF

Posted by GitBox <gi...@apache.org>.
anilmartha commented on pull request #10802:
URL: https://github.com/apache/tvm/pull/10802#issuecomment-1080670783


   @leandron changed the title as per your suggestions.


-- 
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] leandron edited a comment on pull request #10802: Ci fix

Posted by GitBox <gi...@apache.org>.
leandron edited a comment on pull request #10802:
URL: https://github.com/apache/tvm/pull/10802#issuecomment-1080617023


   Hi @anilmartha, thanks for the PR. It looks very promising in fixing https://github.com/apache/tvm/issues/10757.
   
   Do you mind renaming (the tile of the PR) it so that is it more clear for other people not involved in the original issue, what problem this PR is fixing? I would also recommend putting a `[CI]` tag, so that it looks more like other PRs.
   
   cc @ashutosh-arm @driazati @lhutton1 for reviews


-- 
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] leandron commented on pull request #10802: Ci fix

Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #10802:
URL: https://github.com/apache/tvm/pull/10802#issuecomment-1080617023


   Hi @anilmartha, thanks for the PR. It looks very promising in fixing https://github.com/apache/tvm/pull/10802.
   
   Do you mind renaming it so that is it more clear for other people not involved in the original issue, what problem this PR is fixing? I would also recommend putting a `[CI]` tag, so that it looks more like other PRs.
   
   cc @ashutosh-arm @driazati @lhutton1 for reviews


-- 
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] leandron edited a comment on pull request #10802: Ci fix

Posted by GitBox <gi...@apache.org>.
leandron edited a comment on pull request #10802:
URL: https://github.com/apache/tvm/pull/10802#issuecomment-1080617023


   Hi @anilmartha, thanks for the PR. It looks very promising in fixing https://github.com/apache/tvm/issues/10757.
   
   Do you mind renaming it so that is it more clear for other people not involved in the original issue, what problem this PR is fixing? I would also recommend putting a `[CI]` tag, so that it looks more like other PRs.
   
   cc @ashutosh-arm @driazati @lhutton1 for reviews


-- 
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] leandron edited a comment on pull request #10802: Ci fix

Posted by GitBox <gi...@apache.org>.
leandron edited a comment on pull request #10802:
URL: https://github.com/apache/tvm/pull/10802#issuecomment-1080617023


   Hi @anilmartha, thanks for the PR. It looks very promising in fixing https://github.com/apache/tvm/issues/10757.
   
   Do you mind renaming it (the title of the PR) so that is it more clear for other people not involved in the original issue, what problem this PR is fixing? I would also recommend putting a `[CI]` tag, so that it looks more like other PRs.
   
   cc @ashutosh-arm @driazati @lhutton1 for reviews


-- 
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] manupa-arm merged pull request #10802: [CI] Fix Vitis-AI tests when USE_VITIS_AI flag set to OFF

Posted by GitBox <gi...@apache.org>.
manupa-arm merged pull request #10802:
URL: https://github.com/apache/tvm/pull/10802


   


-- 
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] driazati commented on a change in pull request #10802: [CI] Fix Vitis-AI tests when USE_VITIS_AI flag set to OFF

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



##########
File path: tests/python/contrib/test_vitis_ai/test_vitis_ai_codegen.py
##########
@@ -54,6 +54,8 @@ def set_func_attr(func, compile_name, symbol_name):
 )
 def test_conv2d(dpu_target):

Review comment:
       Can you get rid of `skip_test` and use pytest’s skipIf decorator so the skips are properly reported?
   
   https://docs.pytest.org/en/7.1.x/how-to/skipping.html#id1
   




-- 
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] driazati commented on a change in pull request #10802: [CI] Fix Vitis-AI tests when USE_VITIS_AI flag set to OFF

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



##########
File path: tests/python/contrib/test_vitis_ai/test_vitis_ai_codegen.py
##########
@@ -54,6 +54,8 @@ def set_func_attr(func, compile_name, symbol_name):
 )
 def test_conv2d(dpu_target):

Review comment:
       looks good, 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.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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