-
Notifications
You must be signed in to change notification settings - Fork 5.9k
add tensor interfaces #2809
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add tensor interfaces #2809
Conversation
1. Add member variable 'DDim dims_' and a getter function 'dims()'. 'dims' is supposed to hold tensor's shape during Op::InferShape. 2. Remove 'mutable_data' which use default Place. User must specify a explicit Place when call 'mutable_data'. 3. A PlaceHolder may be shared by more than one tensor, and some of them may be the others' slices. So we add a new member variable 'offset_' for Tensor, which is used to show the byte offset between PlaceHolder::ptr_ and where tensor's data really begins. 4. Add functions 'ShareDataFrom' and 'Slice' for Tensor. TODO: Tensor needs a 'CopyFrom' function.
reyoung
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, except tiny little CPP syntax.
paddle/framework/tensor.h
Outdated
| public: | ||
| Tensor() : offset_(0) {} | ||
|
|
||
| Tensor(const DDim& dims) : dims_(dims), offset_(0) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explicit Tensor(const DDim& dims).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
paddle/framework/tensor.h
Outdated
| offset_ = src.offset_; | ||
| } | ||
|
|
||
| Tensor Slice(const int& begin_idx, const int& end_idx) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function could be const
Tensor Slice(int begin_idx, int end_idx) const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
DDim dims_and a getter functiondims().dims_is supposed to hold tensor's shape and it is required byOp::InferShape.mutable_datawhich use defaultPlace. Now users must provide an explicitPlaceas parameter when callingmutable_data. See Proposal: remove get_place and set_place #2742.PlaceHolder::ptr_and where tensor's data really begins.ShareDataFromandSlicefor Tensor.TODO: Tensor needs a 'CopyFrom' function
And there is another question. After
Op::InferShape, we are going to get a series of tensors which have only dims_ but no data(holder_ == nullptr). So it seems reasonable to add a new interface for these tensors' memory allocation:If not, we have to write: