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

Suport Ascend NPU #24277

Closed
wants to merge 3 commits into from
Closed

Suport Ascend NPU #24277

wants to merge 3 commits into from

Conversation

hipudding
Copy link
Contributor

@hipudding hipudding commented Sep 15, 2023

Ascend NPU is a series of AI processors.
hiascend.

This commit make InputArray and OutputArray support a new Mat structor called NpuMat, which has a same interface with Mat and GpuMat. And also, python bindings generator support NpuMat for python interface.

This PR is only for type declaration and code generation. For implementation, please refer to opencv_contrib#3552

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • N/A There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@asmorkalov
Copy link
Contributor

@hipudding Thanks a lot for the contribution!
General comments on the integration:

  • I propose to add Ascend to constant names like NPU_MAT. NPU is too generic and there are several NPU vendors.
  • Most of pages related to Ascend are in Chinese. Please add setup guide to tutorials in English. Also it'll be great to have future proof solution, because it's hard to navigate through pages in Chinese.
  • NPU usage model and restrictions are not clean: what about thread safety, relevant image sizes, etc. Please add some documentation/tutorial about it.
  • It'll be great to have Dockerfile with build environment for CI/Experiments.

@hipudding
Copy link
Contributor Author

@hipudding Thanks a lot for the contribution!

General comments on the integration:

  • I propose to add Ascend to constant names like NPU_MAT. NPU is too generic and there are several NPU vendors.

  • Most of pages related to Ascend are in Chinese. Please add setup guide to tutorials in English. Also it'll be great to have future proof solution, because it's hard to navigate through pages in Chinese.

  • NPU usage model and restrictions are not clean: what about thread safety, relevant image sizes, etc. Please add some documentation/tutorial about it.

  • It'll be great to have Dockerfile with build environment for CI/Experiments.

Sure. Will add them ASAP.

@@ -183,7 +183,8 @@ class CV_EXPORTS _InputArray
#if OPENCV_ABI_COMPATIBILITY < 500
STD_ARRAY =14 << KIND_SHIFT, //!< removed: https://github.com/opencv/opencv/issues/18897
#endif
STD_ARRAY_MAT =15 << KIND_SHIFT
STD_ARRAY_MAT =15 << KIND_SHIFT,
NPU_MAT =16 << KIND_SHIFT
Copy link
Contributor

Choose a reason for hiding this comment

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

For such customization it is better to use approach described here: #20869

Copy link
Contributor Author

@hipudding hipudding Sep 18, 2023

Choose a reason for hiding this comment

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

Thinks for this suggestion. I saw this approach will merge into 5.x. For current release(4.x), Does it has same features to support customer Mat types?
I'd like to use the old way (CUSTOMER_MAT) for 4.x(I don't want to break anything for this stable branch), and refactor it to 5.x after this approach finished. What do you think.

@hipudding
Copy link
Contributor Author

hipudding commented Sep 18, 2023

*I propose to add Ascend to constant names like NPU_MAT. NPU is too generic and there are several NPU vendors.

Change NPU_MAT to ASCEND_MAT.

  • Most of pages related to Ascend are in Chinese. Please add setup guide to tutorials in English. Also it'll be great to
    have future proof solution, because it's hard to navigate through pages in Chinese.

Here's english version website of Ascend(https://www.hiascend.com/en/document), Please have a try and see whether it can provide enough informations.
Tutorials is on the way.

  • It'll be great to have Dockerfile with build environment for CI/Experiments.

Could you give me the visitor's IP address and public key(security information, please send me by email: huafengchun@gmail.com). I will setup a CI VM for opencv, You will be the managers of this machine. And also, Dockerfile is added for compile and run opencv tests on Ascend NPU backend. @see opencv/opencv_contrib#3552

Ascend NPU is a series of AI processors.
[hiascend](https://www.hiascend.com/).

This commit make InputArray and OutputArray support a new Mat
structor called NpuMat, which has a same interface with Mat and GpuMat.
And also, python bindings generator support NpuMat for python interface.
NPU is too generic and there are more than one NPU vendors. It's better
to use ASCEND_MAT instead.
@hipudding
Copy link
Contributor Author

hipudding commented Sep 18, 2023

@asmorkalov @opencv-alalek I found CI is failed due to added/removed symbols here. But I found these symbols is not relate to this PR. Could you give me some suggestions to pass it? Thanks.

the added symbols are added by these PRs: #24028 #23965

@MengqingCao
Copy link
Contributor

  • NPU usage model and restrictions are not clean: what about thread safety, relevant image sizes, etc. Please add some documentation/tutorial about it.

Thanks for your suggestion, the tutorial of Ascend NPU image processing is already available and included in “opencv_contrib/modules/cannops/tutorials/ascend_npu_image_processing.markdown”.

Add a comma to the last element of the enumeration for the future edits.
@hipudding hipudding closed this Nov 3, 2023
@opencv-alalek opencv-alalek removed their request for review November 19, 2023 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants