Remove many usages of Type #21941

li-roy · 2019-06-19T01:19:55Z

Stack from ghstack:

Remove Type dispatch #21964 Remove Type dispatch
Move backward and set_data off of Type #21963 Stop using Type in Python bindings
Remove many usages of Type #21941 Remove many usages of Type
Remove getDeviceFromPtr and allocator from Type #21940 Remove getDeviceFromPtr and allocator from Type
Move UnsafeTensorFromTH and UnsafeStorageFromTH off Type #21923 Move UnsafeTensorFromTH and UnsafeStorageFromTH off Type
[BC-BREAKING] Replace Type dispatch with ATenDispatch #21320 [BC-BREAKING] Replace Type dispatch with ATenDispatch

Removes many usages of type, in preparation for deletion:

Use of Type in python binding for constructors. replaced with TensorTypeId
Use of Type to save python default tensor. replaced with TensorTypeId

Differential Revision: D15893331

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

ezyang · 2019-06-20T13:53:24Z

torch/csrc/autograd/VariableTypeManual.cpp

  for (auto p : backends) {
-    auto baseType = context.getNonVariableTypeRaw(static_cast<Backend>(p), ScalarType::Undefined);
-    if (baseType) {
-      res.emplace_back(VariableType::getVariableTypeFromBaseType(*baseType));


Weird, how come we didn't previously loop? EDIT: Oh, that's because you got rid of scalar type from Type. Makes sense

ezyang · 2019-06-20T13:55:21Z

torch/csrc/tensor/python_tensor.cpp


 static std::vector<PyTensorType> tensor_types;

+void set_default_tensor_type(PyTensorType* type) {


This got moved, are there any substantive changes?

ezyang · 2019-06-20T13:56:02Z

torch/csrc/utils/tensor_types.cpp

        for (int s = 0; s < static_cast<int>(ScalarType::NumOptions); s++) {
-          cpu_map.emplace(type_to_string(*type, static_cast<ScalarType>(s)),
-                          std::make_pair(type, static_cast<ScalarType>(s)));
+          cpu_map.emplace(type_to_string(*type), type);


Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

gchanan · 2019-06-20T14:56:43Z

tools/autograd/templates/VariableType.h

  static bool isVariableType(const at::Type& type);
-  static std::vector<at::Type*> allCUDATypes();
-  static std::vector<at::Type*> allCPUTypes();
+  static std::vector<at::DeprecatedTypeProperties*> allCUDATypes();


is there a plan to not use DeprecatedTypeProperties here?

I think this will soon only be useful for python types. Because we have the concept of FloatTensor and cuda.FloatTensor, we need these to be coupled in some way to convert those strings back and forth. If we get rid of those constructs, we don't need this at all. But we can also get rid of this by just passing around a tuple of TensorTypeId/Backend and ScalarType.

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

gchanan · 2019-06-21T19:33:07Z

torch/csrc/Module.cpp

 PyObject *THPModule_getDefaultDtype(PyObject *_unused, PyObject *arg) {
  HANDLE_TH_ERRORS
-  auto scalar_type = torch::tensors::get_default_scalar_type();
+  auto scalar_type = at::typeMetaToScalarType(torch::tensors::get_default_tensor_options().dtype());


I don't think we should do this.

There is no such thing as default tensor options. There's a default dtype, and a default device type for constructors, but that's it. Using tensor options for this implies there are dimensions of the default which are absolutely not applicable: currently layout, optionality, etc. But soon there will also be dimension names and that doesn't make sense in this context.

The right way to think about this, I think, is that TensorOptions are a mechanism for C++ users to specify kwarg-like options to our factory function. Every other use of them has been a mistake (Type.options(), hacking up our parsing in multiple places to pass TensorOptions instead of just the actual parameters, etc.)

Okay, I'll replace this with get_default_tensor_type_id, which is better than the current get_default_tensor_type because we are still having a default backend, but we just don't have scalar_type info passed along with it.

In the future we should probably have a default device type instead.

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

gchanan · 2019-06-24T15:50:04Z

torch/csrc/utils/tensor_types.cpp

    // torch.cuda. is prefix of str
    std::call_once(cuda_once, []() {
      for (auto type : autograd::VariableType::allCUDATypes()) {
        for (int s = 0; s < static_cast<int>(ScalarType::NumOptions); s++) {


wait, do you still need to loop over ScalarTypes here?

Yeah, we still need to return Backend, ScalarType pair.

but you are never using s?

Oh I see what you mean

gchanan · 2019-06-24T15:50:07Z

torch/csrc/utils/tensor_types.cpp

  } else {
    std::call_once(cpu_once, []() {
      for (auto type : autograd::VariableType::allCPUTypes()) {
        for (int s = 0; s < static_cast<int>(ScalarType::NumOptions); s++) {


gchanan · 2019-06-24T15:57:17Z

test/common_nn.py

        with freeze_rng_state():
            output = test_case._forward(module, input)
-            grad_output = output.new(output.shape).normal_()
+            grad_output = output.new(output.shape, device=output.device).normal_()


is this necessary? If so, that's broken.

gchanan · 2019-06-24T16:42:55Z

aten/src/ATen/detail/CUDAHooksInterface.h

  "PyTorch splits its backend into two shared libraries: a CPU library "
  "and a CUDA library; this error has occurred because you are trying "
-  "to use some CUDA functionality, but the CUDA library has not been "
+  "to use some CUDA functionality, but the CUDA library does not exist."


I can't really parse this change. What are you trying to get across here?

This change isn't needed anymore, I'll revert.

Initially, this PR made it so that this was being hit in some cases when pytorch was being built without CUDA, and I wanted to include that possibility in the error message. But the most recent changes make it so it won't be hit anymore.

gchanan · 2019-06-24T16:55:39Z

c10/core/Backend.h

  }
 }

+static inline Backend backendToBackendOfDeviceType(Backend b, DeviceType d) {


since this only seems to be used in one file (and let's be honest, it's not really clear what it does), can we just move it there instead of having it publicly available?

gchanan · 2019-06-24T16:59:33Z

test/test_torch.py

        self.assertRaisesRegex(AssertionError, msg, lambda: torch.tensor([1], device="cuda"))
        self.assertRaisesRegex(AssertionError, msg, lambda: torch.tensor([1]).cuda())
-        self.assertRaisesRegex(AssertionError, msg, lambda: torch.cuda.FloatTensor())
+        self.assertRaisesRegex(RuntimeError, msg, lambda: torch.cuda.FloatTensor())


why is this one different?

It's hitting a different error now. After most recent changes, it hits an error saying that cuda.FloatTensor isn't available.

gchanan · 2019-06-24T17:40:20Z

torch/csrc/Module.cpp

 PyObject *THPModule_isDefaultTypeCuda(PyObject *_unused, PyObject *arg) {
  HANDLE_TH_ERRORS
-  if (torch::tensors::get_default_tensor_type().is_cuda()) {
+  if (backendToDeviceType(tensorTypeIdToBackend(torch::tensors::get_default_tensor_type_id())) == at::kCUDA) {


not your fault but...this is so crazy, lol.

gchanan · 2019-06-24T17:42:09Z

torch/csrc/autograd/VariableTypeManual.cpp

    }
-    if (!isVariableType(t.dispatch_type())) {
-      AT_ERROR("Expected object of type Variable but found type ", t.dispatch_type().toString(), " at position #", i, " "
+    if (!t.type().is_variable()) {


does it work to do t.is_variable() ?

gchanan · 2019-06-24T17:59:23Z

torch/csrc/tensor/python_tensor.cpp

-  }
-  auto scalar_type = static_cast<ScalarType>(tensor_type.scalar_type);
-  return THPVariable_Wrap(torch::utils::legacy_tensor_ctor(*aten_type, scalar_type, args, kwargs));
+  return THPVariable_Wrap(torch::utils::legacy_tensor_ctor(tensor_type.get_type_id(), tensor_type.get_scalar_type(), args, kwargs));


I don't think your change caused this, but:

>>> torch.randn(2,3).to_mkldnn().new((2,3)) tensor([2., 3.])

is wrong. Do you know what's going on there?

This occurs before my change too, right? If so, I don't have MKLDNN built, but my guess is that new calls legacy_new_from_sequence which calls internal_new_from_data, which uses to, so it doesn't support layouts.

yes, it does. I'll file an issue.

gchanan · 2019-06-24T18:20:00Z

torch/csrc/tensor/python_tensor.cpp

-    throw unavailable_type(*type);
-  }
-  set_default_tensor_type(*aten_type, scalar_type);
+  set_default_tensor_type(type);


I don't know if this is due to this specific change or somewhere else, but this seems worse:

Before you patch (without CUDA):

torch.set_default_tensor_type(torch.cuda.FloatTensor) --------------------------------------------------------------------------- AssertionError Traceback (most recent call last) <ipython-input-2-876b829d82b3> in <module>() ----> 1 torch.set_default_tensor_type(torch.cuda.FloatTensor) /data/users/gchanan/_pytorch11/torch/__init__.py in set_default_tensor_type(t) 151 if isinstance(t, _string_classes): 152 t = _import_dotted_name(t) --> 153 _C._set_default_tensor_type(t) 154 155 /data/users/gchanan/_pytorch11/torch/cuda/__init__.py in _lazy_init() 176 raise RuntimeError( 177 "Cannot re-initialize CUDA in forked subprocess. " + msg) --> 178 _check_driver() 179 torch._C._cuda_init() 180 _cudart = _load_cudart() /data/users/gchanan/_pytorch11/torch/cuda/__init__.py in _check_driver() 90 def _check_driver(): 91 if not hasattr(torch._C, '_cuda_isDriverSufficient'): ---> 92 raise AssertionError("Torch not compiled with CUDA enabled") 93 if not torch._C._cuda_isDriverSufficient(): 94 if torch._C._cuda_getDriverVersion() == 0: AssertionError: Torch not compiled with CUDA enabled

with your patch:

In [4]: torch.set_default_tensor_type(torch.cuda.FloatTensor) In [5]: torch.randn(2,3) --------------------------------------------------------------------------- AssertionError Traceback (most recent call last) <ipython-input-5-1dc2fcf8de67> in <module>() ----> 1 torch.randn(2,3) /data/users/gchanan/_pytorch8/torch/cuda/__init__.py in _lazy_init() 176 raise RuntimeError( 177 "Cannot re-initialize CUDA in forked subprocess. " + msg) --> 178 _check_driver() 179 torch._C._cuda_init() 180 _cudart = _load_cudart() /data/users/gchanan/_pytorch8/torch/cuda/__init__.py in _check_driver() 90 def _check_driver(): 91 if not hasattr(torch._C, '_cuda_isDriverSufficient'): ---> 92 raise AssertionError("Torch not compiled with CUDA enabled") 93 if not torch._C._cuda_isDriverSufficient(): 94 if torch._C._cuda_getDriverVersion() == 0: AssertionError: Torch not compiled with CUDA enabled

fixed and added a test for it.

gchanan · 2019-06-24T18:41:14Z

torch/csrc/utils/tensor_new.cpp

+  maybe_initialize_cuda(type_id);
  AutoNoGIL no_gil;
-  return torch::zeros(sizes, type.options(scalar_type, std::move(device)));
+  if (device.has_value()) {


for all of these that condition based on whether the device exists...can we just build that into your options function?

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

facebook-github-bot · 2019-06-30T11:32:39Z

@li-roy merged this pull request in 9c8f9f0.

Summary: Pull Request resolved: pytorch/pytorch#21941 ghimport-source-id: f20cca6229daba9eb8652adb3d959266ae081ef1 Test Plan: Imported from OSS Differential Revision: D15893331 Pulled By: li-roy fbshipit-source-id: c988b16008ff0e2725a88c6025afd4aabdaca45a

Summary: Pull Request resolved: pytorch#21941 ghimport-source-id: f20cca6 Test Plan: Imported from OSS Differential Revision: D15893331 Pulled By: li-roy fbshipit-source-id: c988b16008ff0e2725a88c6025afd4aabdaca45a

Remove many usages of Type

6d25642

li-roy mentioned this pull request Jun 19, 2019

Move UnsafeTensorFromTH and UnsafeStorageFromTH off Type #21923

Closed

pytorchbot added module: autograd Related to torch.autograd, and the autograd engine in general module: internals Related to internal abstractions in c10 and ATen module: operators module: pybind Related to our Python bindings / interactions with other Python libraries labels Jun 19, 2019

li-roy mentioned this pull request Jun 19, 2019

Remove getDeviceFromPtr and allocator from Type #21940

Closed

Update on "Remove many usages of Type"

00fb469

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

This was referenced Jun 19, 2019

Move backward and set_data off of Type #21963

Closed

Remove Type dispatch #21964

Closed

royboy added 2 commits June 19, 2019 10:46

Update on "Remove many usages of Type"

2642ae0

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

58d1b32

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

li-roy requested review from dzhulgakov, ezyang, gchanan and smessmer June 19, 2019 19:25

ezyang reviewed Jun 20, 2019

View reviewed changes

ezyang approved these changes Jun 20, 2019

View reviewed changes

royboy added 2 commits June 20, 2019 14:05

Update on "Remove many usages of Type"

3c6b7db

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

d3321d5

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

pytorchbot added the module: nn Related to torch.nn label Jun 20, 2019

Update on "Remove many usages of Type"

8cf74d9

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

gchanan reviewed Jun 21, 2019

View reviewed changes

Update on "Remove many usages of Type"

ee7943c

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

gchanan requested changes Jun 21, 2019

View reviewed changes

royboy added 3 commits June 21, 2019 15:27

Update on "Remove many usages of Type"

582eee9

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

042451e

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

b40d998

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

royboy added 5 commits June 21, 2019 15:47

Update on "Remove many usages of Type"

2b01c7f

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

d3579ea

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

ed130d9

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

6b561fb

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

055dd63

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

gchanan reviewed Jun 24, 2019

View reviewed changes

gchanan requested changes Jun 24, 2019

View reviewed changes

Update on "Remove many usages of Type"

ae2b5dc

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

li-roy mentioned this pull request Jun 24, 2019

Remove Type dispatch #22152

Closed

royboy added 3 commits June 24, 2019 12:54

Update on "Remove many usages of Type"

795a596

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

0c1bb64

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

6217ae1

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

li-roy requested a review from gchanan June 25, 2019 01:55

royboy added 7 commits June 25, 2019 01:23

Update on "Remove many usages of Type"

2e4e3eb

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

e9367a0

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

3a5e174

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

9a5a1fe

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

684e9ae

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

8a8c5d9

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

Update on "Remove many usages of Type"

fa036ec

Remove many usages of Type gh-metadata: pytorch pytorch 21941 gh/li-roy/30/head

facebook-github-bot closed this in 9c8f9f0 Jun 30, 2019

zou3519 deleted the gh/li-roy/30/head branch June 30, 2019 11:14

facebook-github-bot added the merged label Jun 30, 2019

mruberry added the Merged label Oct 28, 2020


		static std::vector<PyTensorType> tensor_types;

		void set_default_tensor_type(PyTensorType* type) {

Remove many usages of Type #21941

Remove many usages of Type #21941

Uh oh!

Conversation

li-roy commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ezyang Jun 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

li-roy Jun 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

li-roy Jun 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jun 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

li-roy commented Jun 19, 2019 •

edited

Loading

ezyang Jun 20, 2019 •

edited

Loading

li-roy Jun 21, 2019 •

edited

Loading

li-roy Jun 21, 2019 •

edited

Loading