Agree on this change. Context is more like a region or environment for certain processes. For example, “DispatchContext” and “PassContext” are reasonable naming because they are usually used with with. On the other hand, TVMContext includes just the device information, so it is more like just a data structure.
context is definitely overloaded (can also mean “a collection of state related to a request” e.g. when serving model inference requests).
I do think there are a couple of ways device could be confusing:
- for µTVM, a new user could consider the whole PCB could be a “device,” even though in reality the CPU and accelerator would be separate devices
- perhaps on mobile this would also be true?
I am not really sure if there is a word that would be both concise and not confusing here, though. Executor? Compute Unit?
anyway, is the plan to propose the DLPack change first, then the TVM change to follow?
Yeah, there are different uses of context in the codebase. Device makes more sense to me as well. Would the change to DLPack break other projects that take it as a submodule?
I don’t know much about µTVM. Need to take a look to comment on this.
I think we can make the change in TVM first and change again after the change is pushed to DLPack. Because there’ll be more dependencies in DLPack, it’ll probably take longer time to change in DLPack.
You can also check out and comment on the RFC to DLPack now.
I’m not worried about the µTVM case especially if that’s the only concern. The main thing I’m wondering about is whether it makes sense to let the DLPack RFC get accepted before we make changes here.
I agree with the change, internally it would be nice to drop the TVM prefix as well and just refer to it as a tvm::Device like we do with almost every other data structure in the code base.
Thanks for doing this Haichen!
My take on dropping the TVM prefix: given DLTensor/DLContext is the de facto standard, we do not really need to define their TVM alias (e.g. TVMContext) and can just use them directly
I think we can keep TVMDevice in the C++ and backend, but use tvm.device in the python frontend. It can reduce the confusion when integrating TVM into other frameworks if we keep TVM prefix.
FYI the dlpack RFC Rename DLContext to DLDevice by icemelon9 · Pull Request #62 · dmlc/dlpack · GitHub
1 Like
The dlpack PR is merged now. I will prepare the name change PR in TVM next week. Before that, I want to do a poll on the new name for the TVMContext.
- TVMDevice
- Device
Pro/Cons: TVMDevice can reduce the confusion with the same class name when integrated into frameworks IMO, while Device is just simpler.
1 Like
I vote TVMDevice for consistency
Vote for TVMDevice.
I vote for TVMDevice.
All other TVM primitive types are not prefixed by TVM, it is actually less consistent to use TVM in front of them, currently we have the DL* convention for the low level types and we have tvm::Type for all the high level types including DataType and others.
I think we should prioritize clarity in our code base over integration into others.
1 Like
Because right now TVMContext sits under the global scope(not inside the tvm namespace), as it is needed in the C API.
Another thought is that we could simply use DLDevice instead of TVMDevice to avoid such indirection as apparently we are not introduce a top level abstraction here.
Proposal A0:
Use DLDevice in the C API. Under tvm namespace provid the alias, so we can use Device to refer to it.
// device_api.h
namespace tvm {
using Device = DLDevice;
}
Alternatively, we can simply use DLDevice inside the c++
In the python api we could do tvm.runtime.Device
1 Like
I vote for less aliases, so that’d be using DLDevice everywhere. In the python API, we could call it tvm.runtime.DLDevice to avoid the impression we are renaming it underneath the tvm namespace.
I’d prefer using tvm::Device over directly using DLDevice. DLDevice is not very intuitive for people programming in the TVM repo.
How about we do a new round of poll given the new options. Everyone please share your thoughts
A0
- Use DLDevice in C API
- Introduce tvm::runtime::Device in C++, alias of DLDevice for now, might add member functions(like runtime::DataType’s relation to DLDataType) later
- Introduce tvm.runtime.Device on the python side
Consistency in API with tf.Device and torch.Device. Keep path for future C++ class that has more functionalities(with the same ABI)
A1
- Directly DLDevice in C/C++/python API
Less aliasing. Require future update path for introducing Device class if needed.
I vote for A0 for consistency with DLDataType / tvm::DataType
Vote for A0 +1 to make it more modulized.