Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

fix compute and schedule func of sort and argsoft #1198

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

Shixiaowei02
Copy link
Collaborator

@Shixiaowei02 Shixiaowei02 commented Feb 14, 2023

Fix compute and schedule func of sort and argsoft (first commit in #891) and adding unit tests.

TODO:

  • Remove external calls, do not use local variables, because the size will exceed the limit.
  • Performance optimization.
  • Move decomposer into op mappers.

@Shixiaowei02 Shixiaowei02 changed the title adds the topk operator fix compute and schedule func of sort and argsoft Feb 22, 2023
WhatGhost
WhatGhost previously approved these changes Feb 22, 2023
Copy link
Collaborator

@thisjiang thisjiang left a comment

Choose a reason for hiding this comment

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

应该直接在op mapper中拆分而不是写decomposer,decomposer主要是当初op mapper模块还未加入时的历史遗留模块。而且decomposer还得注册一个算子

cinn/frontend/decomposer/top_k.cc Show resolved Hide resolved
Copy link
Collaborator

@thisjiang thisjiang left a comment

Choose a reason for hiding this comment

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

LGTM

@Shixiaowei02
Copy link
Collaborator Author

Shixiaowei02 commented Feb 22, 2023

应该直接在op mapper中拆分而不是写decomposer,decomposer主要是当初op mapper模块还未加入时的历史遗留模块。而且decomposer还得注册一个算子

在添加算子时,我考虑过这个问题;因为如果希望在 BaseBuilder 中添加一个 TopK,这个算子又不是基础算子的话,拆分逻辑感觉只有 decomposer,目前的几层拆分作用我的理解是:

  1. op_mapper: 最上层的拆分,将 paddle 算子拆分为 cinn::Builder 算子
  2. decomposer: 将 cinn::Builder 组合算子拆分为 cinn::Builder 基础算子 后续此逻辑实现在 cinn::Builder 内部
  3. pe::operation: 将 cinn::Builder 基础算子拆分为 pe 中的 compute + schedule 粒度的算子

@Shixiaowei02 Shixiaowei02 merged commit 0d49470 into PaddlePaddle:develop Feb 22, 2023
Copy link
Collaborator

@thisjiang thisjiang left a comment

Choose a reason for hiding this comment

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

这PR还有bug,后面提的PR里的cinn-ci-x86-mklcblas-off流水都会挂在test_dce_pass这

Shixiaowei02 added a commit to Shixiaowei02/CINN that referenced this pull request Feb 22, 2023
@Shixiaowei02
Copy link
Collaborator Author

Shixiaowei02 commented Feb 22, 2023

这PR还有bug,后面提的PR里的cinn-ci-x86-mklcblas-off流水都会挂在test_dce_pass这

本地和登陆 CI 机器均没能复现,提了一个 PR #1221 ,改日志代码后 CI 问题消失,看起来是随机原因导致,后面继续跟进

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants