Skip to content

Fix GetExpectedKernelType in Concat op#17459

Merged
jerrywgz merged 6 commits intoPaddlePaddle:developfrom
jerrywgz:fix_concat_type
May 23, 2019
Merged

Fix GetExpectedKernelType in Concat op#17459
jerrywgz merged 6 commits intoPaddlePaddle:developfrom
jerrywgz:fix_concat_type

Conversation

@jerrywgz
Copy link
Contributor

Because of #17015, concat op supports empty input now. If the first input is empty, then GetDataTypeOfVar will get Error. In this pr, GetExpectedKernelType is adjusted to get type of the first non-empty input.

@jerrywgz jerrywgz changed the title Fix concat type Fix GetExpectedKernelType in Concat op May 17, 2019
for (size_t i = 0; i < num_input; ++i) {
try {
input_data_type =
framework::GetDataTypeOfVar(ctx.MultiInputVar("X")[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

line 83前先获取下 ctx.MultiInputVar("X")

auto vars = ctx.MultiInputVar("X");
auto input_data_type = paddle::framework::proto::VarType::Type(0);
for (size_t i = 0; i < vars.size(); ++i) {
    try {
      input_data_type = framework::GetDataTypeOfVar(vars[i]);
      ...
}

qingqing01
qingqing01 previously approved these changes May 21, 2019
Copy link
Contributor

@qingqing01 qingqing01 left a comment

Choose a reason for hiding this comment

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

@chengduo Please help to review

PADDLE_THROW("All Inputs of Concat OP are Empty!");
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The above implementation is a bit complicated.
You can try to fix like the following:

auto inputs = ctx.MultiInputVar("X");
for(auto* input : inputs){
  if(input->IsInitialized()){
    ....
  }
}

@CLAassistant
Copy link

CLAassistant commented May 22, 2019

CLA assistant check
All committers have signed the CLA.

@jerrywgz jerrywgz merged commit c1aae8b into PaddlePaddle:develop May 23, 2019
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