Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
【PaddlePaddle Hackathon 3 No.12】为 Paddle 新增 pairwise_distance #44161
【PaddlePaddle Hackathon 3 No.12】为 Paddle 新增 pairwise_distance #44161
Changes from 4 commits
1b7613f
515df56
2c64711
5da6d36
27ebf80
57cec0c
5bfe9de
504a3b6
a523910
34a2575
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
截图来自 PR-CI-APPROVAL
这里利用 _enable_legacy_dygraph 开启 legacy 动态图的原因是原来的 PairwiseDistance(class API)是包含 legacy 动态图代码的,如果不添加相关测试的话,Coverage CI 过不了,因此利用该 API 添加了相关测试,如果该逻辑不再推荐添加的话,我们会删去相关逻辑,并删去相关测试代码
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个到时可以让对应的同学 approve 就好。目前还不能删掉这个逻辑。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好哒~明白~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好的明白。
这里有一个同样的问题,原本的单测(fluid 下面的)也是对 undefined behavior(
[4, 5, 6, 7]
)进行了测试,这里是不是也应该删掉呀~There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好的呀。