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

Move slice to phi #40736

Merged
merged 40 commits into from
Mar 27, 2022
Merged

Move slice to phi #40736

merged 40 commits into from
Mar 27, 2022

Conversation

phlrain
Copy link
Collaborator

@phlrain phlrain commented Mar 20, 2022

PR types

Breaking changes

PR changes

OPs

Describe

move slice to phi

@paddle-bot-old
Copy link

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

}
}

private:
Copy link
Contributor

Choose a reason for hiding this comment

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

这里private下好像没代码,需要删掉吗

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

// the third dimension do not need padding, set padding[2] zero
reshaped_padding[2].first = reshaped_padding[2].second = 0;

LOG(ERROR) << "run here";
Copy link
Contributor

Choose a reason for hiding this comment

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

这个log建议删除了吧

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

DenseTensor* input_grad) {
auto* d_out = &out_grad;
auto* d_input = input_grad;
d_input->mutable_data<T>(ctx.GetPlace());
Copy link
Contributor

Choose a reason for hiding this comment

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

mutable_data -> ctx.Alloc

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

@@ -99,10 +99,11 @@ def config(self):
self.infer_flags = [1, 1, 1]
self.out = self.input[1, 0:3, 2:4, :]

def test_check_output(self):
self.check_output()
# def test_check_output(self):
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


def test_check_grad_normal(self):
print(self.input.size)
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

chenwhql
chenwhql previously approved these changes Mar 25, 2022
Copy link
Contributor

@chenwhql chenwhql left a comment

Choose a reason for hiding this comment

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

细节后续直接PR完善下,后续LoDTensorArray的逻辑也需要迁移,可以参考留杰的stride_slice

place, d_in_t, d_out_t, paddings, static_cast<T>(0));
}
}
private:
Copy link
Contributor

Choose a reason for hiding this comment

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

这里还有一个多余的private

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这个好像没有push上来,我在另外一个pr改下

@phlrain
Copy link
Collaborator Author

phlrain commented Mar 25, 2022

细节后续直接PR完善下,后续LoDTensorArray的逻辑也需要迁移,可以参考留杰的stride_slice

TODO 已记录

ZzSean
ZzSean previously approved these changes Mar 25, 2022
Copy link
Contributor

@ZzSean ZzSean left a comment

Choose a reason for hiding this comment

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

LGTM for op benchmark

@phlrain phlrain dismissed stale reviews from ZzSean and chenwhql via 66ebde2 March 25, 2022 14:46
Copy link
Contributor

@ZzSean ZzSean left a comment

Choose a reason for hiding this comment

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

LGTM for op-benchmark

Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

@phlrain phlrain merged commit b8236b7 into PaddlePaddle:develop Mar 27, 2022
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