-
Notifications
You must be signed in to change notification settings - Fork 26.3k
Created DefaultTensorOptions in ATen #8647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Created DefaultTensorOptions in ATen #8647
Conversation
019b889 to
048e25d
Compare
aten/src/ATen/DefaultTensorOptions.h
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
aten/src/ATen/OptionsGuard.h
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
If you want a design that handles both global and thread local state, I suggest taking a look at https://fb.quip.com/w9G9AlEXbPlq (copy pasted below), which is a proposal of mine on how to put it together: c10: The global contextThis proposal is not slated for immediate implementation (instead, we will simply achieve API parity) MotivationIn most real-world programs, there is a need for some sort of global state which can be easily accessed anywhere in the program. Common examples of such state include:
Global state makes it difficult to have two copies of a library in memory, or run nearby programs with different settings for the context. Thus, there is a desire to encapsulate this state in some sort of context object, to allow for this. Prior ArtPyTorch/ATen. ATen has a Context object, which at present is a single static global variable which user code frequently access directly. Context objects are propagated because every Tensor stores a pointer to the Context object responsible for creating it; “proper” use says that you should use this particular Context object, although a lot of code that was subsequently written for ATen has not followed this constraint. We (Edward, Sam) don't think this model (context passing via Tensor objects) makes semantic sense for what a context object should “be”. Additionally, it seems in practice very difficult to educate developers how to pass around context objects directly, when there is a very attractively named “getGlobalContext()” function which they can use to synthesize a context from scratch. Caffe2. Caffe2 simply uses global variables as necessary. For example, the CPUAllocator is a static unique pointer defined in caffe2/core/allocator.cc; additionally, Caffe2 makes use of gflags (or Caffe2's portable copy thereof), which programs the Caffe2 static registry, which is itself a static library. No context is propagated with actual tensors; instead, per-tensor configuration like the data deleter is stored via the shared_ptr. Design SpaceTo determine how we handle the global context, we have to make a few decisions:
ProposalWe offer the following proposal, given that we answered YES to each of the previous questions. Simpler designs are possible if you decide you don't care about some of these properties. The context is mediated by two levels of indirection: a thread-local context pointer, which points to an atomic memory cell containing a pointer to the actual struct. The Context struct itself is immutable. https://godbolt.org/g/GPZPf6 There are a few key characteristics of the implementation above:
Open questions
|
048e25d to
06b665f
Compare
|
@ezyang I made it thread local, please take another look :) |
|
@pytorchbot retest this please |
a4167cf to
e2e65a4
Compare
e2e65a4 to
82cb0fe
Compare
| /// - layout: kStrided, | ||
| /// - requires_grad: false | ||
| TensorOptions() = default; | ||
| explicit TensorOptions(bool use_thread_local_default_options); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Much better! But ask around about the "early" versus "late" binding of defaults. |
|
This is ready to be merged I think. @ezyang |
Creates a mechanism of globally defaulting
TensorOptionsproperties.The mechanism works primarily via two classes:
DefaultTensorOptions, which provides thread-safe, mutually exclusive access to a single, globalTensorOptionsinstance,OptionsGuard, which will set and reset this globalTensorOptionsinstance.Finally, the default constructor of
TensorOptionswill make a copy of theDefaultTensorOptionsinstance, and copy its properties.Usage is e.g.
Please scrutinize for thread safety and performance.
@colesbury @ezyang @zdevito
CC @ebetica