Skip to content
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

[WIP] Using LoDTensor instead of Tensor in every operator. #4048

Closed
wants to merge 1 commit into from

Conversation

qingqing01
Copy link
Contributor

@qingqing01 qingqing01 commented Sep 12, 2017

Fix #4047
Fix #3717

解决的问题:

@wangkuiyi @reyoung @Superjom @QiJune @hedaoyuan

  • 这里只写了示例程序,还没有全局替换。
  • 和大家讨论下,这种方式是否可以。
  • 如果可行,再全局替换。

"Variable must be type LoDTensor or Tensor");
return *static_cast<const Tensor*>(holder_->Ptr());
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

为了使得不用LoD信息的operators比较方便的从LoDTensor里获取Tensor

  1. 这里特化了Variable::Get<Tensor>接口使得可以
  2. 也可通过方式 https://github.com/PaddlePaddle/Paddle/pull/4048/files#diff-91a47df6a639afa5ad046a8d43c640e0R298 在特化InferShapeContext::Input<Tensor>(const std::string& name)InferShapeContext::Output<Tensor>(const std::string& name) 接口。

{x_mat_dims[0], y_mat_dims[1]});
// Only the forward operator needs to pass the lod.
// pass the lod of Input(X) to output.
ctx.CopyLoD(/*in = */ "X", /* out = */ "Out");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

在每个Op里显示调用LoD传递函数, 解释: #4047 (comment)


void set_tensor(Tensor* tensor) { tensor_ = tensor; }
~LoDTensor() { delete tensor_; }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

LoDTensor管理成员变量Tensor* tensor_的构造与释放,原因:#4047 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe LoDTensor should

  • Disable copy
  • Disable assign
  • Disable move

The simplest way to implement it is make tensor_ as unique_ptr


void set_lod(const LoD& lod) { lod_ = lod; }
LoDTensor(const LoD& lod) : lod_(lod), tensor_(new Tensor()) {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

explicit LoDTensor

@@ -289,13 +289,26 @@ class InferShapeContext {
}

template <typename T>
const T* Input(const std::string& name) const {
virtual const T* Input(const std::string& name) const {
Copy link
Collaborator

Choose a reason for hiding this comment

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

A template method cannot be virtual.

auto* var = InputVar(name);
return var == nullptr ? nullptr : &var->Get<T>();
}

// template <>
Copy link
Collaborator

Choose a reason for hiding this comment

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

??
either delete it or uncomment it

// template <>
// virtual const Tensor* Input<Tensor>(const std::string& name) const {
// auto* var = InputVar(name);
// if (var == nullptr) return nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

delete these

@@ -289,13 +289,26 @@ class InferShapeContext {
}

template <typename T>
const T* Input(const std::string& name) const {
virtual const T* Input(const std::string& name) const {
auto* var = InputVar(name);
return var == nullptr ? nullptr : &var->Get<T>();
Copy link
Contributor

Choose a reason for hiding this comment

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

var ? var : &var->Get<T>()

var is false only if it equals nullptr.

template <typename T>
T* Output(const std::string& name) const {
virtual T* Output(const std::string& name) const {
auto var = OutputVar(name);
return var == nullptr ? nullptr : var->GetMutable<T>();
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

@@ -363,6 +384,27 @@ class ExecutionContext : public InferShapeContext {
return device_context_;
}

template <typename T>
T* Output(const std::string& name) const override {
Copy link
Contributor

Choose a reason for hiding this comment

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

Output be a const T& better?

const T& Output
T* mutable_output
or 
T* MutableOutput

borrowed from Protobuf.

}

template <typename T>
std::vector<T*> MultiOutput(const std::string& name) const override {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above.

T* Output(const std::string& name) const override {
auto var = OutputVar(name);
// Different from InferShapeContext, call Get instread of GetMutable.
return var == nullptr ? nullptr : var->Get<T>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Variable::Get 返回的是一个const reference。Variable::GetMutable返回的是一个mutable pointer。也就是说如要要返回mutable pointer,应该调用的是 GetMutable,而不是Get。这里为什么要调用Get而不是GetMutable呢?

@qingqing01 qingqing01 closed this Sep 15, 2017
@qingqing01 qingqing01 deleted the lod_tensor branch November 14, 2019 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants