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

Enhance reduce op #10708

Merged
merged 11 commits into from
May 23, 2018
Merged

Conversation

wanghaoshuang
Copy link
Contributor

No description provided.

}
int ndim = context.Input<Tensor>("X")->dims().size();
int rdim = context.Attr<std::vector<int>>("dim").size();
HANDLE_DIM(6, 5);
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems we can simplify codes here by employing boost/preprocessor, please refer to https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/operators/expand_op.h#L31, however, it will decrease the readability.

range :math:`[-rank(input), rank(input))`. If :math:`dim < 0`,
the dimension to reduce is :math:`rank + dim`.
range :math:`[-rank(input), rank(input))`. If :math:`dim[i] < 0`,
the dimension to reduce is :math:`rank + dim[i]`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to give the examples when len(dim) > 1 in following Examples 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.

Done. Thx.

@qingqing01 qingqing01 added this to In progress in Computer Vision: OCR May 22, 2018
qingqing01
qingqing01 previously approved these changes May 22, 2018
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.

LGTM.

if (keep_dim && x_rank > 1) {
dims_vector.erase(dims_vector.begin() + dim);
dims = framework::make_ddim(dims_vector);
int DEL_FLAG = -2;
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.

Fixed. Thx.

@@ -126,5 +133,67 @@ def test_check_grad(self):
self.check_grad(['X'], 'Out')


## reduction in multi dims
class TestMeanOpM(OpTest):
Copy link
Contributor

Choose a reason for hiding this comment

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

TestMeanOpM -> TestReduceMeanOpMultiAxises or TestReduceMeanOpMultiDims?

Same as follows.

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. Thx.

1. Rename unitest function.
2. Rename const variable.
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.

LGTM.

@wanghaoshuang wanghaoshuang merged commit 8655904 into PaddlePaddle:develop May 23, 2018
Computer Vision: OCR automation moved this from In progress to Done May 23, 2018
HANDLE_DIM(3, 2);
HANDLE_DIM(3, 1);
HANDLE_DIM(2, 1);
HANDLE_DIM(1, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

reduce_op现在编译特别慢,GPU模式下等了快5分钟才编译完这个op。请问是不是138行-153行支持的维度太多了呢?能否把维度5和6删去,这样就从15种减少到6种了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@panyx0718 merge 个pr减少支持的纬度: #11113

@panyx0718
Copy link
Contributor

@wanghaoshuang @dzhwinter

一起看看#11029
或者想想其他方法解决一下?最近我感觉reduce_op的编译实在是超级慢。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants