Cache isContiguous and numel #10696

cpuhrsch · 2018-08-20T18:53:07Z

No description provided.

facebook-github-bot

cpuhrsch has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

aten/src/ATen/TensorImpl.h

aten/src/TH/THTensor.cpp

aten/src/TH/THTensor.hpp

aten/src/TH/THTensor.cpp

aten/src/TH/THTensor.hpp

aten/src/THC/THCTensor.cpp

ezyang · 2018-08-23T20:16:00Z

The PR is misnamed, it caches numel too

facebook-github-bot

cpuhrsch has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

aten/src/ATen/SparseTensorImpl.cpp

+}
+void SparseTensorImpl::set_storage_offset(int64_t storage_offset) {
+  AT_ERROR("sparse tensors does not have set_storage_offset");
+}


aten/src/ATen/TensorImpl.h

+ protected:
+  void refresh_numel() {
+    int64_t n = 1;
+    for (auto s : sizes()) {


aten/src/ATen/TensorImpl.cpp

+  return numel_;
+}
+
+void TensorImpl::refresh_contiguous() {


aten/src/ATen/TensorImpl.cpp

+  for (auto s : sizes()) {
+    n *= s;
+  }
+  AT_ASSERT(n == numel_);


aten/src/ATen/TensorImpl.h

+  virtual int64_t numel() const;
+
+  virtual bool is_contiguous() const {
+    return is_contiguous_;


ezyang

I don't see anything blocking, and since this has a codemod please feel free to merge. But plenty of things to clean up in follow up diffs! (Or if you're unlucky enough to need to rebase, try to roll some of the suggested changes into this patch.)

aten/src/ATen/TensorImpl.h

+  }
+
+  // WARNING: This function does not check if the requested
+  // sizes/strides are in bounds for the storage is allocated;


aten/src/ATen/TensorImpl.h

+  virtual void resize_dim(int64_t ndim) {
+    // NB: This is *truly* a resize; calling code (e.g., squeeze)
+    // assumes that old values are preserved
+    sizes_.resize(ndim);


aten/src/ATen/TensorImpl.cpp

+}
+
+void TensorImpl::refresh_contiguous() {
+  is_contiguous_ = true;


torch/csrc/autograd/variable.h


  ~Impl() override;

+  int64_t numel() const override;


facebook-github-bot

cpuhrsch has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

cpuhrsch · 2018-08-24T17:05:42Z

Still have to overwrite is_contiguous() in variable::impl

facebook-github-bot

cpuhrsch has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

aten/src/ATen/TensorImpl.h


  // WARNING: This function does not check if the requested
-  // sizes/strides are in bounds for the storage is allocated;
+  // sizes/strides are in bounds bounds for the storage that is allocated;


torch/csrc/autograd/variable.cpp

 }

+bool Variable::Impl::is_contiguous() const {
+  AT_ERROR("variable impl does not have is_contiguous");


aten/src/ATen/SparseTensorImpl.cpp

  AT_ERROR("sparse tensors do not have strides");
 }
+bool SparseTensorImpl::is_contiguous() const {
+  AT_ERROR("sparse tensors does not have is_contiguous");


facebook-github-bot

cpuhrsch has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: Pull Request resolved: pytorch/pytorch#10696 Differential Revision: D9437963 Pulled By: cpuhrsch fbshipit-source-id: 7217682f5e4b69c73d943411d738e4892bb465f5

Summary: Pull Request resolved: pytorch#10696 Differential Revision: D9437963 Pulled By: cpuhrsch fbshipit-source-id: 7217682f5e4b69c73d943411d738e4892bb465f5

cpuhrsch requested review from apaszke, colesbury, ebetica, ezyang, gchanan, goldsborough, pietern, soumith, teng-li and zdevito as code owners August 20, 2018 18:53

cpuhrsch force-pushed the cachecont branch 3 times, most recently from ea4bc82 to 9aa839e Compare August 21, 2018 19:22

facebook-github-bot reviewed Aug 21, 2018

View reviewed changes