Skip to content

Conversation

@yf225
Copy link
Contributor

@yf225 yf225 commented Jun 26, 2019

The original name copy_tensor_data could be confusing because users are not sure whether it deep-copies data in the tensor's storage or just copies the tensor's metadata. The renaming makes it more clear.

cc. @ailzhang This might break XLA build, but I think the renaming makes it more clear why we use copy_tensor_data in XLATensorImpl's shallow-copy functions.

@pytorchbot pytorchbot added module: internals Related to internal abstractions in c10 and ATen oncall: quantization Quantization support in PyTorch labels Jun 26, 2019
@yf225 yf225 requested a review from gchanan June 26, 2019 17:07
@ailzhang
Copy link
Contributor

Thanks! Btw does this make more sense to make it shallow_copy_tensor_metadata since it also copies storage since in our comments we have referred storage and metadata(size/stride...) separately.

@yf225 yf225 force-pushed the rename_copy_tensor_data branch from dc8a5c9 to 78cb4cc Compare June 26, 2019 20:09
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@ailzhang ailzhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making comment consistent!

zdevito pushed a commit to zdevito/ATen that referenced this pull request Jun 27, 2019
Summary:
The original name `copy_tensor_data` could be confusing because users are not sure whether it deep-copies data in the tensor's storage or just copies the tensor's metadata. The renaming makes it more clear.

cc. ailzhang This might break XLA build, but I think the renaming makes it more clear why we use `copy_tensor_data` in XLATensorImpl's shallow-copy functions.
Pull Request resolved: pytorch/pytorch#22266

Differential Revision: D16014724

Pulled By: yf225

fbshipit-source-id: f6ee966927d4d65d828b68264b3253b2f8fd768d
@facebook-github-bot
Copy link
Contributor

@yf225 merged this pull request in 5e0a74d.

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

Labels

Merged module: internals Related to internal abstractions in c10 and ATen oncall: quantization Quantization support in PyTorch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants