-
Notifications
You must be signed in to change notification settings - Fork 26.3k
Create ATen tensors via TensorOptions #7869
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
Conversation
aten/src/ATen/TensorOptions.h
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
bcf2d70 to
e94ff2b
Compare
|
In PyTorch, there is a context-manager which can be used to change the default device options, e.g., changing everything from float to double. Do you plan to eventually support this? If so, with the design as written here you are committing to the fact that these values get read at DeviceOption construction time (since these are direct things, not optional) |
|
That's a very good point. I think it would be easy to implement with thread local globals and a RAII mechanism to set defaults in a particular scope. I would personally think it's fine that values are fixed at the call site (where you write |
|
Why does TensorOptions have backend and integral device? Why not have device objects (like python) and get rid of backend (which can be inferred from device and layout)? |
|
That's Coming |
1aec84c to
1b6813b
Compare
755fa3e to
4e3cb37
Compare
d5645f6 to
c7040a5
Compare
aten/src/ATen/Layout.cpp
Outdated
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.
aten/src/ATen/function_wrapper.py
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
ezyang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ship it already yo
zdevito
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some details about TensorOptions like the underlying Type* field that we might want to figure out better solutions for. However, this PR is getting to big, so assuming there are not any known bugs, I think we should merge and revisit those nits afterwards.
bb97ef5 to
87159de
Compare
Storing the type in TensorOptions to solve the Variable problem
Created convenience creation functions for TensorOptions and added tests
Converted zeros to TensorOptions
Converted rand to TensorOptions
Fix codegen for TensorOptions and multiple arguments
Put TensorOptions convenience functions into torch namespace too
All factory functions except *_like support TensorOptions
Integrated with recent JIT changes
Support *_like functions
Fix in place modification
Some cleanups and fixes
Support sparse_coo_tensor
Fix bug in Type.cpp
Fix .empty calls in C++ API
Fix bug in Type.cpp
Trying to fix device placement
Make AutoGPU CPU compatible
Remove some auto_gpu.h uses
Fixing some headers
Fix some remaining CUDA/AutoGPU issues
Fix some AutoGPU uses
Fixes to dispatch_tensor_conversion
Reset version of new variables to zero
Implemented parsing device strings
Random fixes to tests
Self review cleanups
flake8
Undo changes to variable.{h,cpp} because they fail on gcc7.2
Add [cuda] tag to tensor_options_cuda.cpp
Move AutoGPU::set_index_from into .cpp file because Windows is stupid and sucks
Fix linker error in AutoGPU.cpp
Fix bad merge conflict in native_functions.yaml
Fixed caffe2/contrib/aten
Fix new window functions added to TensorFactories.cpp
Added code to generate wrapper functions for factory methods Add implicit constructor from Backend to TensorOptions Remove Var() from C++ API and use torch:: functions Use torch:: functions more subtly in C++ API Make AutoGPU::set_device more exception safe Check status directly in DynamicCUDAHooksInterface Rename AutoGPU to DeviceGuard Removed set_requires_grad from python_variables.h and warn appropriately in Variable::set_requires_grad remove python_default_init: self.type() Add back original factory functions, but with deprecation warnings Disable DeviceGuard for a couple functions in ATen Remove print statement Fix DeviceGuard construction from undefined tensor Fixing CUDA device compiler issues Moved as many methods as possible into header files Dont generate python functions for deprecated factories Remove merge conflict artefact Fix tensor_options_cuda.cpp Fix set_requires_grad not being checked Fix tensor_new.h TEMPORARILY put some methods in .cpp files to see if it solves issues on windows and mac Fix bug in DeviceGuard.h Missing includes TEMPORARILY moving a few more methods into .cpp to see if it fixes windows Fixing linker errors
Undo device agnostic behavior of DeviceGuard Use -1 instead of optional for default device index Also move DeviceGuard methods into header Fixes around device index after optional -> int32_t switch Fix use of DeviceGuard in new_with_tensor_copy Fix tensor_options.cpp
87159de to
d7b2814
Compare
|
|
||
| // Guard GPU device | ||
| AutoGPU gpuGuard; | ||
| at::DeviceGuard gpuGuard; |
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.
| } else { | ||
| backend = (layout_ == kStrided) ? kCUDA : kSparseCUDA; | ||
| } | ||
| return getType(backend, dtype_); |
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.
This PR is a prototype / WIP for a broad change to the way tensors are created in ATen. The fundamental motivation for this PR is to make tensor creation from C++ more like in Python, which means changing the API for factory functions like
at::ones, as well as teaching ATen about construction axes beyond the backend and scalar type, meaning especially adevice. This change essentially lays the ground for how tensor creation will happen in c10, and thus defines a new API, currently bolted on top of existing ATen.This PR proposes the following:
Creation of a type encapsulating all construction axes of a tensor
At the moment, this type will contain:
And in the near future shall contain (and already does somewhat in this prototype):
requires_gradboolean.in code:
Each of these fields have default values, such that a default-constructed
TensorOptionsobject fully specifies a new tensor.Furthermore, this class shall have chaining methods:
and there shall be functions with the names of fields, that return a new
TensorOptionsinstance:Use of
TensorOptionsin all factory methodsThe new API for tensor creation shall then be (e.g. for
ones):with usage examples such as
New code path
This PR also implements a new code path for factory methods. For our recollection, at the moment, we create tensors via e.g.
which dispatches to
which itself calls
which then calls
This PR proposes to remove factory methods from
Type, and skip the dispatch viaTypealtogether for all factory methods excepttensor, which remains the fundamental source of tensors invoked by all other tensor factories:now directly calls
which now does
At the moment, I have implemented this for
ones, and I think nothing broke. There should be no implications for the Python API.@zdevito @apaszke @gchanan @colesbury @ezyang
Fixes #6285
Fixes #6286
Fixes #7735