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/02/09 15:10:51 UTC

[GitHub] [tvm] NicolaLancellotti opened a new pull request #7427: Make the TVM targets list available in Python

NicolaLancellotti opened a new pull request #7427:
URL: https://github.com/apache/tvm/pull/7427


   This PR allows to get the names of the available targets in Python 


----------------------------------------------------------------
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] junrushao1994 commented on pull request #7427: Make the TVM targets list available in Python

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


   I agree with @leandron and @comaniac. Perhaps a good way is to have both `list_kinds` and `list_tags` correspondingly


----------------------------------------------------------------
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] junrushao1994 edited a comment on pull request #7427: Make the TVM targets list available in Python

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


   Thanks for the discussion, and sorry for the late reply!
   
   To clarify:
   - Target `tag`s are an alias and an efficient way to create concrete targets. For example, `nvidia/rtx2080ti` is short for "cuda -arch sm_75 -shared_memory_per_block 49152 -registers_per_block 65536 -thread_warp_size 32".
   - Target `kind`s mean specifically the codegen target, i.e. do we codegen to LLVM IR (where the kind is "llvm"), or CUDA C (where the kind is "cuda"), etc. We write `kind` as the first term in the target string.
   


----------------------------------------------------------------
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] NicolaLancellotti commented on pull request #7427: Make the TVM targets list available in Python

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


   I renamed "ListAllNames" to "ListTargetKinds" and "names" to "list_kinds".


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   @junrushao1994 please take a final look to approve and merge.


----------------------------------------------------------------
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] leandron edited a comment on pull request #7427: Make the TVM targets list available in Python

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


   Thanks for the replies @comaniac @zxybazh. One final thing from me (to get you on the right timezone) - just so that we update gwith a potential final version of the PR.
   
   Any preferences on the Python-side function? At the moment I see it is called `Target.names()`. Shall we change it to  `Target.kinds()` for example, or even to something else?


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   I agree. For the function name selection, I would prefer to use `ListTargetKind(s)` as @junrushao1994 suggested, somehow more consistent with the meaning of `kind`. `Names` still seem a bit vague for me, but I would also be fine with 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] NicolaLancellotti commented on pull request #7427: Make the TVM targets list available in Python

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


   Please, @mbaret can you review?


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   On Python-side function, I think `names` is a bit vague. Instead, `kinds` is more clear and consistent with the other `libs` function name.


----------------------------------------------------------------
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] junrushao1994 merged pull request #7427: Make the TVM targets list available in Python

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


   


----------------------------------------------------------------
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] leandron edited a comment on pull request #7427: Make the TVM targets list available in Python

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


   Thanks for the replies @comaniac @zxybazh. One final thing from me (to get you on the right timezone) - just so that we update with a potential final version of the PR.
   
   Any preferences on the Python-side function? At the moment I see it is called `Target.names()`. Shall we change it to  `Target.kinds()` for example, or even to something else?


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   Oh, I see. It's following the `list_tags` function in `tag.py`. Sounds good to me.


----------------------------------------------------------------
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] junrushao1994 commented on pull request #7427: Make the TVM targets list available in Python

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


   To clarify, we have `ListTags` to list all the target tags, and this PR further adds the functionality to list target kinds. Is my understanding correct? Thanks a lot!


----------------------------------------------------------------
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] junrushao1994 commented on pull request #7427: Make the TVM targets list available in Python

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


   Thanks for the discussion, and sorry for the late reply!
   
   To clarify:
   - Target `tag`s are an alias and an efficient way to create concrete targets. For example, `nvidia/rtx2080ti` is short for "cuda -arch sm_75 -shared_memory_per_block 49152 -registers_per_block 65536 -thread_warp_size 32".
   - Target `kind`s mean specifically the codegen target, i.e. do we codegen to LLVM IR (where the kind is "llvm"), or CUDA C (where the kind is "cuda"), etc.
   


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   IMHO, it would be better to use a more consistant name. i.e., `list_kinds`. This also aligns to the naming convension of `list_tags`.


----------------------------------------------------------------
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] leandron commented on pull request #7427: Make the TVM targets list available in Python

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


   Thanks for the replies @comaniac @zxybazh. One final thing (to get you on the right timezone) - just so that we update gwith a potential final version of the PR.
   
   Any preferences on the Python-side function? At the moment I see it is called `Target.names()`. Shall we change it to  `Target.kinds()` for example, or even to something else?


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   I think the PR is just to demonstrate the supported backend types, which is called `kind`, as @comaniac mentioned. Though accessible via code, a function would be helpful. `tag` is only for the usage of a complete target configuation like pre-defined tags or json target string.


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   On Python-side function, I think `names` is a bit vague. I think `kinds` is more clear and consistent with the `libs` function name.


----------------------------------------------------------------
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] leandron commented on pull request #7427: Make the TVM targets list available in Python

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


   > On Python-side function, I think `names` is a bit vague. Instead, `kinds` is more clear and consistent with the other `libs` function name.
   
   I think @comaniac makes a good point for `list_kinds()` - would you agree with that @zxybazh? If so, we’ll update it tomorrow with the resulta of this discussion.


----------------------------------------------------------------
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] leandron commented on pull request #7427: Make the TVM targets list available in Python

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


   > I think the PR is just to demonstrate the supported backend types, which is called `kind`, as @comaniac mentioned. Though accessible via code, a function would be helpful. `tag` is only for the usage of a complete target configuation like pre-defined tags or json target string.
   
   Yes, I can confirm we would like - from this PR - a list of str that contains all `kind` of targets, such as llvm, cuda, opencl, for validation and better error messages on TVMC.
   
   I think the current proposal is not creating a lot of overhead.
   
   My suggestion is to get agreement on the proper name for the function, unless there is a duplication of functionality with something already in the code base - what do you think?


----------------------------------------------------------------
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] mbaret commented on pull request #7427: Make the TVM targets list available in Python

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


   > Therefore, to keep things consistent, do you think we should be more specific here and say like `ListTargetKind` instead of using `names`?
   
   I would be slightly concerned that the user expectation for ListTargetKind(s) would be a list of TargetKind type, rather than a list of string names. Perhaps `ListTargetKindNames` instead?
   


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   Agree on the ListTargetKinds in this case. ListTargetKindNames doesn't make it more clear to me.


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   Ah I messed up with `tag.cc` and `target_kind.cc`...
   
   So basically `kind` refers to the backend, such as `llvm`, `cpu`, `gpu`, `cuda`, and `tag` refers to a complete target string or JSON, such as `cuda -keys=cuda,gpu -arch=sm_61 -max_num_threads=1024 -thread_warp_size=32`. Since target kind is less used and I supposed this PR is for TVMC, I assume TVMC users should mostly just use tags to make sure everything is covered.
   


----------------------------------------------------------------
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] leandron commented on pull request #7427: Make the TVM targets list available in Python

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


   > I reviewed the changes again based on the discussion. Why don't we just add a syntactic sugar`Target.list_tags().keys()` in the Python side so that we don't need a new function?
   
   I might be wrong, but I was looking into the sources and trying to understand this. When  you mention `list_tags()`, are you referring to `tvm.target.tag.list_tags()`?
   
   If so, I'm not getting what I expected to get from there, is that a bug?
   ```
   >>> tvm.target.tag.list_tags()
   {"nvidia/gtx1080ti": cuda -keys=cuda,gpu -arch=sm_61 -max_num_threads=1024 -thread_warp_size=32, "nvidia/rtx2080ti": cuda -keys=cuda,gpu -arch=sm_75 -max_num_threads=1024 -thread_warp_size=32}
   ```
   
   @junrushao1994, what is the conceptual difference between `kind` and `tag`, in the target jargon?


----------------------------------------------------------------
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] junrushao1994 commented on pull request #7427: Make the TVM targets list available in Python

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


   Also CC: @zxybazh 


----------------------------------------------------------------
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 #7427: Make the TVM targets list available in Python

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


   I reviewed the changes again based on the discussion. Why don't we just add a syntactic sugar`Target.list_tags().keys()` in the Python side so that we don't need a new 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] [tvm] comaniac edited a comment on pull request #7427: Make the TVM targets list available in Python

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


   IMHO, it would be better to use a more consistant name. i.e., `list_kinds`. This also aligns to the naming convention of `list_tags`.


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