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/04/26 10:22:18 UTC

[GitHub] [tvm] pfk-beta opened a new pull request, #11126: refactoring - move devices-related stuff from tvm.runtime.ndarray to …

pfk-beta opened a new pull request, #11126:
URL: https://github.com/apache/tvm/pull/11126

   Hello,
   
   I'm not sure whether it needs RFC, but it was little messy in python runtime code (related to my answer, not main thread in this post https://discuss.tvm.apache.org/t/pre-rfc-more-meaningful-names-in-rpc-module-in-python/12618). I have moved `device, cpu, cuda, gpu, opencl, cl, vulkan, metal, mtl, vpi, rocm, ext_dev` functions from `tvm.runtime.ndarray` to `tvm.runtime.devices` and reimported them in `tvm.runtime`, so we can use following statement:
   ```
   import tvm.runtime
   runtime.cpu(0)
   ```
   
   In previous version there was mix of following:
   ```
   import tvm
   tvm.cpu()
   ```
   
   ```
   from tvm.runtime import ndarray as _nd
   _nd.cuda(0)
   ```
   


-- 
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 pull request #11126: [PY] refactoring - move devices-related functions from tvm.runtime.ndarray

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

   sorry, i meant `tvm.cuda`


-- 
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 pull request #11126: [PY] refactoring - move devices-related functions from tvm.runtime.ndarray

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

   Thanks @pfk-beta , the original rationale was that device is closely coupled with ndarray itself(as ndarray is the only places that device get allocated). 
   
   Re-exposing the devices under tvm.runtime namespace would indeed be a readability improvement. From UX's pov we intentionally expose tvm.cpu/tvm.cuda not for internal development but for users, so we would like to keep those exposures to avoid API breaking.
   
   For the same reason(although weaker as there are less usages here), we would like to keep `nd.device` usages when possible. Personally i think putting devices definition as separate file or in ndarray do not have a big difference, although in this case it does bring a minor readability gain. 
   
   One simple approach is to re-expose nd.cpu etc in the runtime namespace, and use runtime.device in most of the places you mention.  This would be the least surgical change while retains the readability part.
   
   
   
   
   


-- 
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 pull request #11126: [PY] refactoring - move devices-related functions from tvm.runtime.ndarray

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

   Thanks @pfk-beta . I agree this is a positive change towards readability. Please check the linter error(all new files needs ASF header). The overall convention of using `tvm.runtime.device` sounds good. Although I do not think we should prevent developers from using `tvm.device` itself as this is also what intended for the users


-- 
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] pfk-beta commented on pull request #11126: [PY] refactoring - move devices-related functions from tvm.runtime.ndarray

Posted by GitBox <gi...@apache.org>.
pfk-beta commented on PR #11126:
URL: https://github.com/apache/tvm/pull/11126#issuecomment-1110808221

   > Re-exposing the devices under tvm.runtime namespace
   
   You mean devices - functions, not module which I created, right? They were already exposed, I just standrized imports and usage of them. Even python api documentation list these functions in tvm.runtime (https://tvm.apache.org/docs/reference/api/python/runtime.html)
   
   > From UX's pov we intentionally expose tvm.cpu/tvm.cuda not for internal development but for users, so we would like to keep those exposures to avoid API breaking.
   
   That is why, I didn't remove devices functions from top `tvm` module. I think we should put this informations in guideline for python developers, which want to contribute to TVM. Because I refactored mix of usage, and I don't want do this again :) Is there such guideline, or should we create one?


-- 
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] pfk-beta commented on pull request #11126: [PY] refactoring - move devices-related functions from tvm.runtime.ndarray

Posted by GitBox <gi...@apache.org>.
pfk-beta commented on PR #11126:
URL: https://github.com/apache/tvm/pull/11126#issuecomment-1117428682

   > Although I do not think we should prevent developers from using `tvm.device` itself as this is also what intended for the users
   
   To clarify, you mean using `tvm.device.cuda` or `tvm.cuda`?
   
   


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