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

[Ascend] use aclnnUnique2 impl unique #1314

Merged
merged 8 commits into from
Jul 30, 2024

Conversation

hellozmz
Copy link
Contributor

Motivation and Context

[Ascend] use aclnnUnique2 impl unique

Description

Use cases (Optional)

BC-breaking (Optional)

Checklist

Before PR:

  • I have read and followed the workflow indicated in the Contributors.md to create this PR.
  • Pre-commit or linting tools indicated in Contributors.md are used to fix the potential lint issues.
  • Bug fixes are covered by unit tests, the case that causes the bug should be added in the unit tests.
  • New functionalities are covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  • The documentation has been modified accordingly, including docstring or example tutorials.

After PR:

  • CLA has been signed and all committers have signed the CLA in this PR.

@hellozmz hellozmz self-assigned this Jul 19, 2024
hellozmz added 2 commits July 19, 2024 14:37
@hellozmz hellozmz marked this pull request as ready for review July 19, 2024 08:37
@hellozmz hellozmz requested a review from jingguo-st July 19, 2024 08:37
diopiError_t diopiUnique(diopiContextHandle_t ctx, diopiTensorHandle_t* out, diopiConstTensorHandle_t input, const int64_t* dim, bool sorted, bool returnCounts,
diopiTensorHandle_t indices, diopiTensorHandle_t* counts) {
// aclnnUnique2 only support dim == nullptr
ASCEND_CHECK_ABORT(dim == nullptr, "dim is not supported in aclnnUnique2");
Copy link
Collaborator

@yangbofun yangbofun Jul 22, 2024

Choose a reason for hiding this comment

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

华为有aclnnUniqueDim来调用 带dim的unique.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

华为有aclnnUniqueDim来调用 带dim的unique. image

嗯嗯,尝试用aclnnUniqueDim去实现支持dim的场景,不过有些测试用例没有通过,看模型中暂时使用的是unique2,先实现了不使用dim的算子,并且用ASCEND_CHECK_ABORT检查了dim。

Copy link
Collaborator

Choose a reason for hiding this comment

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

华为有aclnnUniqueDim来调用 带dim的unique. image

嗯嗯,尝试用aclnnUniqueDim去实现支持dim的场景,不过有些测试用例没有通过,看模型中暂时使用的是unique2,先实现了不使用dim的算子,并且用ASCEND_CHECK_ABORT检查了dim。

在pr描述里写一下,并且在ASCEND_CHECK_ABORT这里写上可以调用aclnnUniqueDim.

// allocate temp inverse tensor
diopiTensorHandle_t inverseTmp = nullptr;
if (returnInverse || returnCounts) {
diopiRequireTensor(ctx, &inverseTmp, &inSize, nullptr, diopi_dtype_int64, diopi_device);
Copy link
Collaborator

@yangbofun yangbofun Jul 22, 2024

Choose a reason for hiding this comment

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

参考camb的requiresTensor写一些函数通用函数(华为上叫makeTensor),这样就不用每次都从diopiSize_t来获取tensor

@yangbofun yangbofun merged commit 6246289 into DeepLink-org:main Jul 30, 2024
17 checks passed
hellozmz added a commit to DeepLink-org/DIOPI.dev that referenced this pull request Aug 2, 2024
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.

5 participants