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.

Sign up for GitHub

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

Jump to bottom

update the code for the topk v2, test=develop #26494

Merged
merged 13 commits into from
Aug 25, 2020
Merged

update the code for the topk v2, test=develop #26494

merged 13 commits into from
Aug 25, 2020

Conversation

wawltor
Copy link
Contributor

@wawltor wawltor commented Aug 20, 2020 •
edited
Loading

PR types

Others

PR changes

OPs

Describe

Add the new topk v2 for the paddlepaddle api 2.0

  • 新增功能
  1. 排序axis支持，既有版本只支持对Tensor的最后一维进行TopK操作，新增版本可以通过设置axis，可以对Tensor其它的维度进行TopK操作。
  2. 排序功能，既有版本对TopK返回的结果是排序之后的结果，新增版本可以设置标志来控制返回的结果是否排序，对返回结果不进行排序可以加快算子的计算速度。用户可以根据需要来设定。
  3. 控制返回结果升序还是降序，既有版本只能返回K个最大值，而新增版本可以返回K个最大值，也可以返回K个最小值，用户可以根据需要来设置相应的标志。
  • 不兼容功能
  1. NaN在比较过程中的行为定义，目前既有版本对NaN值处理方式是一个不确定行为，导致在取Tensor的TopK的结果包含随机性，在合理情况下应该确定NaN值在比较过程中的行为，目前常用数学计算包和其他深度学习框架都是认为NaN值是最大的，因此新增TopK版本确定了NaN值在比较过程中的行为，作为最大值处理。
    例如 input = [1, NaN, 1], 进行Top 3的抽取，
    既有PaddlePaddle的TopK，返回的是 [1, NaN, 1]
    合理的返回的的结果是 [NaN, 1, 1]
  • 新增V2版本原因
    由于新增版本在同等的参数的算子行为和既有版本的行为不一致，如果在原始代码修改会造成既有模型在推理阶段的不兼容，因此需要新增V2版本。

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

XiaoguangHu01
XiaoguangHu01 previously approved these changes Aug 21, 2020
View reviewed changes
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

python/paddle/tensor/search.py Outdated
If the input is a Tensor with higher rank, this operator computes the top k values and indices along the :attr:`axis`.

Args:
x(Tensor): Tensor, an input N-D Tensor with type float32, float64, int32, int6.
Copy link
Contributor

Choose a reason for hiding this comment

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

int64

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 it

TCChenlong
TCChenlong previously approved these changes Aug 22, 2020
View reviewed changes
Copy link
Contributor

@TCChenlong TCChenlong left a comment

Choose a reason for hiding this comment

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

LGTM

raindrops2sea
raindrops2sea requested changes Aug 24, 2020
View reviewed changes
paddle/fluid/operators/top_k_v2_op.h
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License. */

Copy link
Collaborator

Choose a reason for hiding this comment

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

请注释补充一下新增V2实现的原因考量，和原来版本的区别。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

had add it

XiaoguangHu01
XiaoguangHu01 previously approved these changes Aug 24, 2020
View reviewed changes
raindrops2sea
raindrops2sea approved these changes Aug 24, 2020
View reviewed changes
Copy link
Collaborator

@raindrops2sea raindrops2sea left a comment

Choose a reason for hiding this comment

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

LGTM

TCChenlong
TCChenlong approved these changes Aug 25, 2020
View reviewed changes
Copy link
Contributor

@TCChenlong TCChenlong left a comment

Choose a reason for hiding this comment

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

LGTM

XiaoguangHu01
XiaoguangHu01 approved these changes Aug 25, 2020
View reviewed changes
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

@wawltor wawltor merged commit 286eca2 into PaddlePaddle:develop Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XiaoguangHu01 XiaoguangHu01 XiaoguangHu01 approved these changes

@raindrops2sea raindrops2sea raindrops2sea approved these changes

@TCChenlong TCChenlong TCChenlong approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wawltor @TCChenlong @raindrops2sea @XiaoguangHu01