Use variable_data() in tensor_to_numpy #22214
Conversation
pytorchbot
added
module: internals
torch/csrc/utils/tensor_numpy.cpp
Outdated
| @@ -110,7 +115,7 @@ PyObject* tensor_to_numpy(const at::Tensor& tensor) { | |||
| // object of the ndarray to the tensor and disabling resizes on the storage. | |||
| // This is not sufficient. For example, the tensor's storage may be changed | |||
| // via Tensor.set_, which can free the underlying memory. | |||
| PyObject* py_tensor = THPVariable_Wrap(make_variable(tensor, false)); | |||
| PyObject* py_tensor = THPVariable_Wrap(as_variable_ref(tensor).variable_data()); | |||
There was a problem hiding this comment.
We could add an explicit check for tensor.is_variable() here, but I think as_variable_ref(tensor) already checks whether tensor is a Variable.
There was a problem hiding this comment.
Do you need the variable_data?
There was a problem hiding this comment.
I just checked and we don't need to use variable_data here - resizing is not supported on either the PyTorch tensor or the numpy array, so creating a shallow-copy of the tensor being converted is not necessary in any case.
There was a problem hiding this comment.
@yf225 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| @@ -85,6 +85,11 @@ PyObject* tensor_to_numpy(const at::Tensor& tensor) { | |||
| if (tensor.type().backend() != Backend::CPU) { | |||
| throw TypeError("NumPy conversion for %s is not supported", tensor.type().toString().c_str()); | |||
| } | |||
| if (tensor.requires_grad()) { | |||
| throw std::runtime_error( | |||
| "Can't call numpy() on Variable that requires grad. " | |||
There was a problem hiding this comment.
This error message is outdated. There are no more Variables in PyTorch.
There was a problem hiding this comment.
There are in C++ still, see #13638. We should fix these once that proposal is implemented, though.
As part of the Variable/Tensor merge, we want to gradually remove call sites of
tensor_data()and the API itself, and instead usesvariable_data(). This PR removes thetensor_data()call in the tensor_to_numpy conversion path.