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 2021/04/02 16:19:11 UTC

[GitHub] [tvm] zxybazh opened a new pull request #7791: [Target] Fix empty target and host for autotvm task

zxybazh opened a new pull request #7791:
URL: https://github.com/apache/tvm/pull/7791


   In order to patch the empty target and host situation in [Issue 7790](https://github.com/apache/tvm/issues/7790).


-- 
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] [tvm] zxybazh commented on pull request #7791: [Target] Fix empty target and host for autotvm task

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


   Hi Cody, I don't intend to add test for the function because this is a temporary function for migration to the new target system. It should be deprecated soon after target host deprecation and is not encouraged for any other usage. I have also added the assertion as per your good point. Please review and let me know any other advice, 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] [tvm] zxybazh commented on pull request #7791: [Target] Fix empty target and host for autotvm task

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


   Thanks @comaniac for pointing that out. I have added check in this case. In order to prevent this issue, I would take a look at other occurances as well and update in this PR.


-- 
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] [tvm] zxybazh commented on pull request #7791: [Target] Fix empty target and host for autotvm task

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






-- 
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] [tvm] comaniac commented on pull request #7791: [Target] Fix empty target and host for autotvm task

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


   > Thanks @comaniac for pointing that out. I have added check in this case. In order to prevent this issue, I would take a look at other occurances as well and update in this PR.
   
   Sure. Please comment here once it's ready for reivew.


-- 
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] [tvm] tqchen merged pull request #7791: [Target] Fix empty target and host for autotvm task

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


   


-- 
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] [tvm] comaniac commented on a change in pull request #7791: [Target] Fix empty target and host for autotvm task

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



##########
File path: python/tvm/target/target.py
##########
@@ -182,6 +182,10 @@ def check_and_update_host_consist(target, host=None, target_is_dict_key=True):
         target_is_dict_key : Bool
             When the type of target is dict, whether Target is the key (Otherwise the value)
         """
+        if target is None:
+            if host is not None:
+                warnings.warn("Target host is not empty when target is empty.")

Review comment:
       Should this be a warning or an assertion, if target=None and host!=None doesn't make any sense?




-- 
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] [tvm] zxybazh commented on a change in pull request #7791: [Target] Fix empty target and host for autotvm task

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



##########
File path: python/tvm/target/target.py
##########
@@ -182,6 +182,10 @@ def check_and_update_host_consist(target, host=None, target_is_dict_key=True):
         target_is_dict_key : Bool
             When the type of target is dict, whether Target is the key (Otherwise the value)
         """
+        if target is None:
+            if host is not None:
+                warnings.warn("Target host is not empty when target is empty.")

Review comment:
       Good point, should be an assertion. I fixed 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.

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



[GitHub] [tvm] comaniac commented on a change in pull request #7791: [Target] Fix empty target and host for autotvm task

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



##########
File path: python/tvm/target/target.py
##########
@@ -182,6 +182,9 @@ def check_and_update_host_consist(target, host=None, target_is_dict_key=True):
         target_is_dict_key : Bool
             When the type of target is dict, whether Target is the key (Otherwise the value)
         """
+        if target is None:
+            assert host is None

Review comment:
       You can keep the message to let people know why it's failed.




-- 
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] [tvm] zxybazh commented on pull request #7791: [Target] Fix empty target and host for autotvm task

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


   Well, it does impact a lot of functions. I have added tests and fixed assertion warning as suggested. Please check, 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] [tvm] zxybazh edited a comment on pull request #7791: [Target] Fix empty target and host for autotvm task

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


   Thanks @comaniac for pointing that out. I have added check in this case. In order to prevent this issue, I would take a look at other occurrences as well and update in this PR.


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