Skip to content

Replace interactive batched Matrix Multiply. #24812

Merged
asmorkalov merged 3 commits intoopencv:4.xfrom
Abdurrahheem:ash/einsum_bachedGemm
Jan 12, 2024
Merged

Replace interactive batched Matrix Multiply. #24812
asmorkalov merged 3 commits intoopencv:4.xfrom
Abdurrahheem:ash/einsum_bachedGemm

Conversation

@Abdurrahheem
Copy link
Copy Markdown
Contributor

@Abdurrahheem Abdurrahheem commented Jan 4, 2024

This PR replaces iterative batch matrix multiplication which FastGemmBatch in Einsum layer.

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
  • 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

@Abdurrahheem Abdurrahheem self-assigned this Jan 4, 2024
fengyuentau
fengyuentau reviewed Jan 5, 2024
View reviewed changes
modules/dnn/src/layers/einsum_layer.cpp

std::vector<Mat> output;
Mat reshapedInput1 = input1;
Mat reshapedInput2 = input2;
Copy link
Copy Markdown
Member

@fengyuentau fengyuentau Jan 5, 2024

Choose a reason for hiding this comment

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

Could you elaborate why reshape is needed here? fastGemmBatch can already handle multiplification of n-dimensional matrices.

Copy link
Copy Markdown
Contributor Author

@Abdurrahheem Abdurrahheem Jan 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is required as some tensors might come in with dummy deletion BxNxMx1. Reshape just removes that dummy dimension.

Copy link
Copy Markdown
Member

@fengyuentau fengyuentau Jan 6, 2024

Choose a reason for hiding this comment

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

Could you give a real Einsum example of dummy deletion BxNxMx1?

Copy link
Copy Markdown
Contributor Author

@Abdurrahheem Abdurrahheem Jan 10, 2024

Choose a reason for hiding this comment

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

This dummy dimension on BxNxMx1 appears during pre-processing for some tensors. The reshape is not needed because we want to multiply 2 tenors with dummy dimension.

Copy link
Copy Markdown
Contributor Author

@Abdurrahheem Abdurrahheem Jan 10, 2024

Choose a reason for hiding this comment

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

@fengyuentau is there anything to change more? Could you please approve if not ?

asmorkalov
asmorkalov approved these changes Jan 6, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

👍

@asmorkalov asmorkalov added this to the 4.10.0 milestone Jan 6, 2024
@asmorkalov asmorkalov marked this pull request as ready for review January 9, 2024 17:06
@Abdurrahheem
Copy link
Copy Markdown
Contributor Author

Abdurrahheem commented Jan 10, 2024

@dkurt any comments?

dkurt
dkurt reviewed Jan 10, 2024
View reviewed changes
@asmorkalov asmorkalov merged commit c923c59 into opencv:4.x Jan 12, 2024
This was referenced Jan 19, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fengyuentau fengyuentau fengyuentau left review comments

@asmorkalov asmorkalov asmorkalov approved these changes

@dkurt dkurt dkurt approved these changes

Assignees

@Abdurrahheem Abdurrahheem

Labels

category: dnn

Projects

None yet

Milestone

4.10.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Abdurrahheem @asmorkalov @fengyuentau @dkurt