Skip to content

add all_kernels_must_compute_runtime_shape example for speedup infershape#16154

Merged
luotao1 merged 5 commits intoPaddlePaddle:developfrom
luotao1:infershape_example
Mar 13, 2019
Merged

add all_kernels_must_compute_runtime_shape example for speedup infershape#16154
luotao1 merged 5 commits intoPaddlePaddle:developfrom
luotao1:infershape_example

Conversation

@luotao1
Copy link
Contributor

@luotao1 luotao1 commented Mar 11, 2019

We plan to implement computing runtime shape for all ops, in order to skip runtime infershape for speedup.

This PR add all_kernels_must_compute_runtime_shape attributes and make an example of three ops: fused_embedding_seq_pool_op, hash_op and sequence_enumerate_op.

Related #15959

@luotao1 luotao1 requested review from Xreki and panyx0718 March 12, 2019 02:08
// and skip infershape in runtime for speedup.
// TODO(luotao): Note that it is a temporal attribute, after all ops
// implement computing runtime shape, this attribute would be deleted.
if (!HasAttr("all_kernels_must_compute_runtime_shape")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use a constant for the string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"(boolean, default true) "
"An attribute to speed up OperatorWithKernel::RunImpl."
"If true, all the kernels of this Op would compute runtime "
"shape, but skip infershape in runtime. Note that it is a temporal "
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Reorganize the sentences.

test=develop
@luotao1
Copy link
Contributor Author

luotao1 commented Mar 13, 2019

[19:22:57][Step 1/1] current pr 16154 got approvals: FALSE
[19:22:57][Step 1/1] You must have panyx0718 approval for the api change! paddle/fluid/framework/operator.h
[19:22:57][Step 1/1] + APPROVALS=FALSE
[19:22:57][Step 1/1] + echo 'current pr 16154 got approvals: FALSE'

});
AddAttr<int>("pad_value", "(int) The enumerate sequence padding value.")
.SetDefault(0);
AddAttr<bool>(framework::kAllKernelsMustComputeRuntimeShape, "")
Copy link
Contributor

Choose a reason for hiding this comment

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

doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since doc of kAllKernelsMustComputeRuntimeShape is so long and the same for all the ops, do you think we should put the doc in each op?

Currently, the doc is in operator.h: https://github.com/PaddlePaddle/Paddle/pull/16154/files#diff-f93a864675e978ec63f28873aa308a5cR71

@luotao1 luotao1 merged commit 4ef6f73 into PaddlePaddle:develop Mar 13, 2019
@luotao1 luotao1 deleted the infershape_example branch March 13, 2019 16:01
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.

2 participants