Merge TensorImpl. #11971

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.

Sign up for GitHub

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

Jump to bottom

Closed

ezyang wants to merge 16 commits into export-D9967509 from 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