Skip to content

Conversation

@reyoung
Copy link
Collaborator

@reyoung reyoung commented Sep 3, 2017

Fix #3822

const Variable* InputVar(const std::string& name) const {
return scope_.FindVar(op_.Input(name));
auto ipt = op_.Input(name);
if (ipt == kEmptyVarName) {
Copy link
Member

Choose a reason for hiding this comment

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

如果在scope中检查如果name是kEmptyVarName就报个错,是不是不用加这么多代码

Copy link
Member

Choose a reason for hiding this comment

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

不过现在这种方式比较便于理解代码逻辑

const Variable* InputVar(const std::string& name) const {
return scope_.FindVar(op_.Input(name));
auto ipt = op_.Input(name);
if (ipt == kEmptyVarName) {
Copy link
Collaborator

@JiayiFeng JiayiFeng Sep 3, 2017

Choose a reason for hiding this comment

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

using

return ipt == kEmptyVarName ? nullptr : scope_.FindVar(ipt);

can make the code shorter.

PADDLE_THROW("Op %s input %s should contain only one variable", type_,
name);
return "";
}
Copy link
Collaborator

@JiayiFeng JiayiFeng Sep 3, 2017

Choose a reason for hiding this comment

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

std::string OperatorBase::Input(const std::string& name) const { auto& ins = Inputs(name); PADDLE_ENFORCE(ins.size() <= 1, "....."); return ins.empty() ? kEmptyVarName : ins[0]; }

The code is much shorter.

PADDLE_THROW("Op %s output %s should contain only one variable", type_,
name);
return "";
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

See my comment on OperatorBase::Input().

@jacquesqiao
Copy link
Member

是不是应该在construct op的时候检查input output参数列表是否完整?

@reyoung reyoung force-pushed the feature/fix_empty_input_and_output branch from ae85ed2 to d7a1e40 Compare September 3, 2017 19:04
@reyoung
Copy link
Collaborator Author

reyoung commented Sep 3, 2017

@jacquesqiao @Canpio All things done.

jacquesqiao
jacquesqiao previously approved these changes Sep 3, 2017
Copy link
Member

@jacquesqiao jacquesqiao left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@JiayiFeng JiayiFeng left a comment

Choose a reason for hiding this comment

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

LGTM

@reyoung reyoung merged commit c1feb27 into PaddlePaddle:develop Sep 4, 2017
lcy-seso pushed a commit to lcy-seso/Paddle that referenced this pull request Sep 4, 2017
…nput_and_output Make operator Input/Output can return nullptr
@reyoung reyoung deleted the feature/fix_empty_input_and_output branch October 2, 2017 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants