BlobGetMutableTensor returns Tensor (#14136) #14424
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smessmer
approved these changes
Nov 27, 2018
caffe2/core/blob.h
Outdated
| @@ -28,6 +28,7 @@ inline Tensor* BlobSetTensor(Blob* blob, const Tensor& tensor) { | |||
| return blob->Reset<Tensor>(new Tensor(tensor)); | |||
| } | |||
|
|
|||
| // need to keep both for clangr codemod | |||
There was a problem hiding this comment.
nit: maybe specify what you mean by "both"
jerryzh168
force-pushed
the
export-D12934074
branch
from
dfb5614
to
66f1d12
Compare
Summary: Pull Request resolved: pytorch#14424 Pull Request resolved: pytorch#14136 Since now Tensor is a shared_ptr, it doesn't make sense to have Tensor* around anymore, so we want to change Tensor* to Tensor in the interface. We added functions that work with `Tensor` instead of `Tensor*` in this diff. To remove Tensor*, we'll do following ``` auto* Y = Ouptut(0); Y->mutable_data... ``` --> ``` auto Y = Output(0); Y.mutable_data... ``` But to run clangr codemod, we'll keep both APIs in different names, e.g. `Output` and `XOutput`, and do the refactor and then delete the old method and rename the new method into the old one. For example for `Output`, we'll first codemod the callsites from `Output` to `XOutput`, then delete the old `Output` and rename `XOutput` to `Output` in the end. Reviewed By: smessmer Differential Revision: D12934074 fbshipit-source-id: 120778830835fc4d90286cf2ed00b4994cf32737
jerryzh168
force-pushed
the
export-D12934074
branch
from
66f1d12
to
8bb14df
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Pull Request resolved: #14136
Since now Tensor is a shared_ptr, it doesn't make sense to have Tensor* around anymore,
this diff is changing Tensor* to Tensor in the interface.
We'll store pointer to intrusive_ptr to TensorImpl in Tensor and create Tensor object on the fly
when returning from BlobGetMutableTensor.
Another motivation is we want Blob to store intrusive_ptr of TensorImpl because IValue can store intrusive ptr. Without this change, IValue will have to store Blob which is an extra level of indirection.
To remove Tensor*, we'll do following
-->
But to run clangr codemod, we'll keep both APIs in different names, e.g.
OutputandXOutput, and do the refactor and then delete the old method and rename the new method into the old one.For example for
Output, we'll first codemod the callsites fromOutputtoXOutput, then delete the oldOutputand renameXOutputtoOutputin the end.This diff should be rebased on the diffs that removes Resize/ResizeLike+mutable_data
Differential Revision: D12934074