[RFC] Rename TVMContext to TVMDevice

haichen · February 7, 2021, 5:46am

I propose to rename the TVMContext to TVMDevice. Currently, TVMContext is used to represent a device that the model is executed on. Two main reasons for this change:

the name of TVMContext doesn’t intuitively reflect its semantics of a device.
mainstream frameworks including TensorFlow and PyTorch both uses device to represent the device to run. I think it’d be good that we use the similar terminology to reduce the confusion.

Let me know your thoughts. @tqchen @jroesch @zhiics @comaniac @junrushao

junrushao · February 7, 2021, 10:52am

Thanks for bringing this up!

Context is indeed a bad name when it is used to indicate a certain device, because its meaning really varies depending on the “context”. In this case, “device” is certainly an acceptable name and is a lot better than “context”.

Two thoughts:

when referring to a remote device in TVM RPC, what word should we use? Right now we still use ctx.
in dlpack, we still use DLContext, so are we going to propose a name change in dlpack as well?

haichen · February 8, 2021, 7:25am

TVM uses RPCSession to create a remote context/device. We can also change this API to device.
In fact, I also plan to create a RFC to dlpack for the context name change.

junrushao · February 8, 2021, 7:49am

Yeah this change sounds quite reasonable to me. BTW, the DLPack case might be a bit intricate, because it needs consensus across different frameworks, but let’s leave the discussion to DLPack RFC

comaniac · February 8, 2021, 5:40pm

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.

areusch · February 8, 2021, 8:56pm

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?

zhiics · February 9, 2021, 1:06am

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?

haichen · February 9, 2021, 1:40am

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.

areusch · February 9, 2021, 5:35pm

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.

jroesch · February 9, 2021, 7:11pm

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!

junrushao · February 9, 2021, 7:30pm

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

haichen · February 9, 2021, 9:19pm

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.

tqchen · February 19, 2021, 9:02pm

FYI the dlpack RFC Rename DLContext to DLDevice by icemelon9 · Pull Request #62 · dmlc/dlpack · GitHub

haichen · February 20, 2021, 12:16am

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.

@tqchen @jroesch @junrushao @comaniac @areusch @zhiics

areusch · February 20, 2021, 12:42am

I vote TVMDevice for consistency

comaniac · February 20, 2021, 12:45am

Vote for TVMDevice.

Johnson9009 · February 20, 2021, 1:05am

I vote for TVMDevice.

jroesch · February 24, 2021, 9:52am

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.

tqchen · February 25, 2021, 2:15pm

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

areusch · February 25, 2021, 4:48pm

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.