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

Refactor DDim's product() and add slice_ddim() #2900

Merged
merged 5 commits into from
Jul 17, 2017

Conversation

JiayiFeng
Copy link
Collaborator

  1. Refactor DDim's product() to make it more efficiently.
  2. Add slice_ddim().

fix #2880

1. Refactor DDim's product() to make it more efficiently.

2. Add slice_ddim().
@JiayiFeng JiayiFeng requested review from luotao1 and QiJune July 16, 2017 07:23
@@ -81,6 +81,8 @@ std::vector<int> vectorize(const DDim& ddim);

ssize_t product(const DDim& ddim);

DDim slice_ddim(const DDim& dim, int begin, int end);
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
Collaborator Author

Choose a reason for hiding this comment

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

done


DDim slice_ddim(const DDim& dim, int begin, int end) {
std::vector<int> vec;
vec.reserve(end - begin);
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to check the value of begin and end here?
such as

begin <= end;
begin>=1;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We do these checks in SliceVectorizeVisitor.

SliceVectorizeVisitor(std::vector<int>& v, int b, int e)
      : vector(v), begin(b), end(e) {
    PADDLE_ENFORCE(begin < end,
                   "Begin index must be less than end index in ddim slice.");
    PADDLE_ENFORCE(begin >= 0,
                   "Begin index can't be less than zero in ddim slice.");
  }

and

void operator()(const Dim<1>& dim) {
    PADDLE_ENFORCE(end == 1, "End index in ddim slice is out of bound.");
    vector.push_back(dim.head);
  }

for (auto i : v) {
result *= i;
ProductVisitor visitor;
return boost::apply_visitor(visitor, ddim);
Copy link
Member

@QiJune QiJune Jul 17, 2017

Choose a reason for hiding this comment

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

I know that a Dim<D> variable can be passed to a DDim variable.
But, how a DDim varibale passed to a Dim<D> variable?

Copy link
Collaborator Author

@JiayiFeng JiayiFeng Jul 17, 2017

Choose a reason for hiding this comment

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

See https://github.com/Canpio/Paddle/wiki/Majel:-From-Allocation-to-DArray
In the last section, I show how DArray is casted into Array in boost::apply_visitor. The principle is also suitable for DDim and Dim.

Copy link
Member

@QiJune QiJune left a comment

Choose a reason for hiding this comment

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

LGTM

}
}

void operator()(const Dim<1>& dim) {
Copy link
Member

Choose a reason for hiding this comment

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

This implementation is a little tricky. And all implementation related with dim is tricky.
We may consider remove dim later and just use a small vector.

@JiayiFeng JiayiFeng merged commit a33e9da into PaddlePaddle:develop Jul 17, 2017
@JiayiFeng JiayiFeng deleted the dev_ddim_update branch July 17, 2017 06:44
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.

Add slice function for dim
3 participants