Merge TensorImpl.#11971

Closed

ezyang wants to merge 16 commits intoexport-D9967509from

export-D9995559

Contributor

ezyang commented Sep 22, 2018 •

edited

Loading

Stack:
:white_circle: #11970 Make ATen-core and caffe2 mutually recursive / merge template data() 💚
:black_circle: #11971 Merge TensorImpl. 💚

Switched TensorImpl::data() to use Storage::unsafe_data() to work
around an outstanding bug in the Storage::data() implementation
where it only works on Ts which are valid ScalarType
Qualify a bunch of identifiers which still live in caffe2:: namespace
strides returns an IntList now
s/update_strides/update_to_contiguous_strides/
Correctly compute type_id_ for the Storage only constructor from Caffe2.
This is special cased to only work for CPU and CUDA dense tensors.
Fix some signed-unsigned comparisons in Caffe2 code (OSS build for
ATen/core has more restrictive warning tests.)

Differential Revision: D9995559


          V6: Initial commit

22315e0

Differential Revision: D9995559
Differential Version: 58616425

This was referenced Sep 22, 2018

Make ATen-core and caffe2 mutually recursive / merge template data<T>() #11970

Closed

Merge UndefinedTensorImpl. #11972

Closed

Merge Tensor. #11973

Closed

ezyang added 7 commits

September 22, 2018 04:59


          V7: sign-compare fix

f5b3312

Differential Revision: D9995559
Differential Version: 58617218


          V8: (no description)

504cfbf

Differential Revision: D9995559
Differential Version: 58630166


          V9: (no description)

f548074

Differential Revision: D9995559
Differential Version: 58630679


          V10: Fix another unsigned signed comparison

feaf464

Differential Revision: D9995559
Differential Version: 58638144


          V11: Merge with parent diff changes

Differential Revision: D9995559
Differential Version: 58638317


          V12: (no description)

961751b

Differential Revision: D9995559
Differential Version: 58649580


          V13: (no description)

Differential Revision: D9995559
Differential Version: 58650202

ezyang commented

View reviewed changes

caffe2/core/tensor.h

               class CAFFE2_API UndefinedTensorImpl final : public TensorImpl {
-                UndefinedTensorImpl() : TensorImpl(at::Storage()){};
+                UndefinedTensorImpl() : TensorImpl(CPU){};

This comment was marked as off-topic.

Sign in to view

ezyang added 2 commits

September 24, 2018 09:10


          V14: Merge with parent diff changes

f3eeaef

Differential Revision: D9995559
Differential Version: 58689897


          V16: Merge with parent diff changes

dffccb1

Differential Revision: D9995559
Differential Version: 58739233

dzhulgakov reviewed

View reviewed changes

Collaborator

dzhulgakov left a comment

wow, we're very close!

aten/src/ATen/core/TensorImpl.h

+                  auto newCapacity = sizes_;
+                  newCapacity[0] = std::max<size_t>(
+                      newDims[0], std::ceil(sizes_[0] * (growthPct + 100) / 100));
+                  auto oldData = std::move(storage_.data_ptr());

This comment was marked as off-topic.

Sign in to view


          V17: Merge with parent diff changes

ab945a5

Differential Revision: D9995559
Differential Version: 58826237

gchanan reviewed

View reviewed changes

aten/src/ATen/core/StorageImpl.h

                 template <typename T>
                 inline T* data() const {
+                  // TODO: This is bad: it means storage.data<T>() calls only work on

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/core/TensorImpl.h

+              // A global boolean variable to control whether we free memory when a Tensor
+              // is shrinked to a smaller size. As a result, a Tensor is always going to
+              // keep the memory allocated for its maximum capacity reshaped to so far.
+              CAFFE2_DECLARE_bool(caffe2_keep_on_shrink);

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/core/TensorImpl.h

+              /**
+               * A utility function to convert vector<int> to vector<int64_t>.
+               */
+              inline std::vector<int64_t> ToVectorint64_t(const std::vector<int>& src) {

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/core/TensorImpl.h Outdated

+               * in Caffe2 that stores a contiguous memory with its shape information.
+               *
+               * The TensorImpl class is essentially a wrapper around a device-specific memory
+               * (the device is specified by the Context template argument), and deals with

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/core/TensorImpl.h Outdated

+               *
+               * The TensorImpl class is essentially a wrapper around a device-specific memory
+               * (the device is specified by the Context template argument), and deals with
+               * the allocation and de-allocation of such memory. We make a simplified

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/core/TensorImpl.h

                 TensorImpl(TensorTypeId type_id, const caffe2::TypeMeta& data_type, Allocator *allocator, bool is_variable);
                 TensorImpl(Storage&& storage, TensorTypeId type_id, bool is_variable);
+                explicit TensorImpl(at::Storage storage) : storage_(std::move(storage)), storage_offset_(0) {

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/core/TensorImpl.h Outdated

+                /*
+                 * Since we removed template from tensor, we now store a static
+                 * context pointer in tensor, which indicates the type of the tensor.

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/core/TensorImpl.h

+                    storage_ = at::Storage(GetDeviceType(), src.meta());
+                    data_type_ = src.meta();
+                  }
+                  if (src.size() == -1) {

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/core/TensorImpl.h

+                  }
+                  if (src.size() == -1) {
+                    sizes_.clear();
+                    numel_ = -1;

This comment was marked as off-topic.

Sign in to view

aten/src/ATen/core/TensorImpl.h

+                 * items is the same, the underlying storage is kept.
+                 */
+                template <typename... Ts>
+                void Resize(Ts... dim_source) {

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

ezyang added 2 commits

September 26, 2018 07:37


          V18: Update comments

c1b6954

Differential Revision: D9995559
Differential Version: 58921808


          V19: Merge with parent diff changes

Differential Revision: D9995559
Differential Version: 58923260

ezyang mentioned this pull request

Rename TensorImpl::GetDeviceType to device_type, and properly test if is_variable #12087

Closed


          V20: bugfix

8ca0fb6

Differential Revision: D9995559
Differential Version: 58923468

This was referenced Sep 26, 2018

Move ExtendTo to caffe2::Tensor from TensorImpl #12089

Closed

Move TensorImpl::ShrinkTo to caffe2::Tensor #12090

Closed

Move TensorImpl::ResizeLike to caffe2::Tensor #12091

Closed

Fix bug where Reshape() trashes strides. #12092

Closed

Move TensorImpl::Reshape(vector<int>) to caffe2::Tensor #12094

Closed

Move TensorImpl::DebugString() to caffe2::Tensor #12095

Closed

Move TensorImpl::ShareExternalPointer helper overloads to caffe2::Tensor #12096

Closed

Implement caffe2::Tensor::raw_data() in terms of data() #12097

Closed

Move TensorImpl ndim, size, itemsize and nbytes to caffe2::Tensor #12098

Closed

This was referenced Sep 26, 2018

Move TensorImpl size_from_dim, size_to_dim, size_between_dim, canonical_axis_index to caffe2::Tensor #12099

Closed

Move TensorImpl IsType, meta, dim32, dim, ExtractDeviceOption to caffe2::Tensor #12100

Closed


          V22: Merge with parent diff changes

3c7c696

Differential Revision: D9995559
Differential Version: 59091385

jerryzh168 reviewed

View reviewed changes

aten/src/ATen/core/TensorImpl.h

+                  return storage_.unsafe_data<T>() + storage_offset_;
                 }
                 inline void* data() const {

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

jerryzh168 approved these changes

View reviewed changes

Contributor

jerryzh168 left a comment

Approved via ghapprove -s D10051424


          V23: Accept numel_ == -1 too

d312c42

Differential Revision: D9995559
Differential Version: 59124130

facebook-github-bot closed this in

f6abd16

zdevito pushed a commit to zdevito/ATen that referenced this pull request


          Merge TensorImpl. (#11971)

ba975fb

Summary:
Pull Request resolved: pytorch/pytorch#11971

- Switched TensorImpl::data<T>() to use Storage::unsafe_data<T>() to work
  around an outstanding bug in the Storage::data<T>() implementation
  where it only works on Ts which are valid ScalarType
- Qualify a bunch of identifiers which still live in caffe2:: namespace
- strides returns an IntList now
- s/update_strides/update_to_contiguous_strides/
- Correctly compute type_id_ for the Storage only constructor from Caffe2.
  This is special cased to only work for CPU and CUDA dense tensors.
- Fix some signed-unsigned comparisons in Caffe2 code (OSS build for
  ATen/core has more restrictive warning tests.)

Reviewed By: jerryzh168

Differential Revision: D9995559

fbshipit-source-id: 9c74032e011189e1c7e9a98d20f2bd1e25ad2e5c

soumith deleted the export-D9995559 branch

February 21, 2019 23:26

ezyang added the merged label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet