Skip to content

Add matrix multiplication to Mat for iOS/Android#20731

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
komakai:matrix_mult_android_ios
Oct 5, 2021
Merged

Add matrix multiplication to Mat for iOS/Android#20731
opencv-pushbot merged 1 commit intoopencv:masterfrom
komakai:matrix_mult_android_ios

Conversation

@komakai
Copy link
Copy Markdown
Contributor

@komakai komakai commented Sep 22, 2021 •
edited by alalek
Loading

Pull Request Readiness Checklist

  • 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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There are tests

Android and iOS wrapper both have element-wise multiplication operations (https://en.wikipedia.org/wiki/Hadamard_product_(matrices)) for Mat but lack matrix multiplication operations (https://en.wikipedia.org/wiki/Matrix_multiplication).

This PR adds matrix multiplication for Mat in Android and iOS wrappers.
It also adds operator overloading for the * operator in Swift and Kotlin

force_builders=Custom Mac
build_image:Custom Mac=osx_framework-test
buildworker:Custom Mac=macosx-2

alalek
alalek reviewed Sep 22, 2021
View reviewed changes
modules/core/misc/java/src/java/core+Mat.java Outdated
* Matrix multiplication
*/
public Mat matrixMul(Mat m) {
return new Mat(n_matrixMul(nativeObj, m.nativeObj));
Copy link
Copy Markdown
Member

@alalek alalek Sep 22, 2021

Choose a reason for hiding this comment

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

What is wrong with gemm()?

Copy link
Copy Markdown
Contributor Author

@komakai komakai Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The problem I have with gemm is it takes 3 Mat as input. If you want to just simply multiply 2 matrices - you need to create a third completely unnecessary Mat just to do the operation. Also the name gemm makes it difficult to find without hunting around on Stack Overflow.

Copy link
Copy Markdown
Member

@alalek alalek Oct 4, 2021

Choose a reason for hiding this comment

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

"GEMM" name comes from BLAS.

Also the name gemm makes it difficult to find

BTW,

Perhaps I would suggest to replace matrixMul -> matMul as OpenCV already have cv::Mat instead of cv::Matrix.

/cc @vpisarev @asmorkalov

Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov Oct 5, 2021

Choose a reason for hiding this comment

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

+1 for matMul

@komakai komakai force-pushed the matrix_mult_android_ios branch from e49d67d to 30955b3 Compare September 23, 2021 10:06
@komakai komakai force-pushed the matrix_mult_android_ios branch 2 times, most recently from 39737f9 to eb44528 Compare September 26, 2021 08:52
@komakai komakai force-pushed the matrix_mult_android_ios branch from eb44528 to 19a880b Compare October 5, 2021 11:16
@komakai
Copy link
Copy Markdown
Contributor Author

komakai commented Oct 5, 2021

Changed method name from matrixMul to matMul and added some links in the documentation to Core.gemm

alalek
alalek approved these changes Oct 5, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Well done! Thank you 👍

@opencv-pushbot opencv-pushbot merged commit 13c6eb4 into opencv:master Oct 5, 2021
@alalek alalek mentioned this pull request Oct 15, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov asmorkalov left review comments

@alalek alalek alalek approved these changes

Assignees

@alalek alalek

Labels

category: core category: java bindings platform: ios/osx

Projects

None yet

Milestone

4.5.4

Development

Successfully merging this pull request may close these issues.

4 participants

@komakai @asmorkalov @alalek @opencv-pushbot