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.
I vote for A0 after careful consideration. DLDevice requires extra knowledge of DLPack
I think I originally proposed A1 but one thing is that we have the TVM extension device types, so perhaps keeping a separate Device is better. I’m ok w/ A0.
Since everyone agree on A0, I’ll go ahead and prepare the PR in next week.
1 Like
PR is ready for review now.