DNN: make GEMM can be supported with transA and transB in CUDA by WanliZhong · Pull Request #23061 · opencv/opencv

WanliZhong · 2022-12-29T09:25:46Z

Merge with: opencv/opencv_extra#1033

This PR tries to make GEMM can be supported with transA and transB in CUDA. It's a follow-up to #22882
GEMM onnx documentation: https://github.com/onnx/onnx/blob/main/docs/Operators.md#Gemm

Because MatMul can be seen as a specific case of GEMM, I put the GEMM into MatMulOp. This may make it possible to implement the whole GEMM operation: αAB+βC in the future.

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

force_builders=Custom
buildworker:Custom=linux-1
docker_image:Custom=ubuntu-cuda:16.04

zihaomu

LGTM! 👍

alalek · 2023-01-19T08:05:05Z

modules/dnn/src/onnx/onnx_importer.cpp

    }

    layerParams.set("bias_term", node_proto.input_size() == 3);
+    layerParams.set("is_matmul", true);


is_matmul

Do we really need this?
Where is it used?

Does InnerProduct without is_matmul make sense (used somewhere)?

Such layer parameters should be described in headers' documentation, because this behavior is not obvious.

In the current implementation, InnerProduct and MatMul are implemented using the same code. Previously, it used to distinguish the two by judging whether InputB is const or not, which could not distinguish the two in some cases, so the is_matmul parameter was introduced.

I add this parameter in #22828. If it is acceptable, I will add some description in header's documentation.

Why importer should know about that implementation details?

Interfaces should be kept clear and stay implementation agnostic.
Implementation should use inheritance instead of adding of new strange external parameters.

Because the implementation cannot tell whether the operator is matrix multiplication or inner product, it needs to be informed by the importer.

If it is just 2D matrix multiplication, it can share the same code with inner product, but high-dimensional matrix multiplication cannot be implemented with inner product code. I haven't thought a better way to distinguish them than separating the implementation part of the matrix multiplication from the inner product.

cannot tell whether the operator is matrix multiplication or inner product, it needs to be informed by the importer

It is done by name/type of the layer: layerParams.type = "InnerProduct";

Thanks! This is really useful! I will remove this parameter later.

It is done by name/type of the layer: layerParams.type = "InnerProduct";

This solution can‘t be implemented to distinguish MatMul, GEMM and InnerProduct, because their layerParams.type are all InnerProduct. They all use the fully_connected_layer to implement. I still have to specific a parameter in importer.

MatMul use layerParams.type = "InnerProduct":

opencv/modules/dnn/src/onnx/onnx_importer.cpp

Lines 2063 to 2067 in 606c803

void ONNXImporter::parseMatMul(LayerParams& layerParams, const opencv_onnx::NodeProto& node_proto_)

{

opencv_onnx::NodeProto node_proto = node_proto_;

CV_Assert(node_proto.input_size() == 2);

layerParams.type = "InnerProduct";

GEMM use layerParams.type = "InnerProduct":

opencv/modules/dnn/src/onnx/onnx_importer.cpp

Lines 2001 to 2004 in 606c803

void ONNXImporter::parseGemm(LayerParams& layerParams, const opencv_onnx::NodeProto& node_proto)

{

CV_Assert(node_proto.input_size() >= 2);

layerParams.type = "InnerProduct";

Other operators:

opencv/modules/dnn/src/caffe/caffe_io.cpp

Lines 1069 to 1070 in 606c803

case V1LayerParameter_LayerType_INNER_PRODUCT:

return "InnerProduct";

opencv/modules/dnn/src/darknet/darknet_io.cpp

Lines 181 to 185 in 606c803

cv::dnn::LayerParams getParamFullyConnected(int output)

{

cv::dnn::LayerParams params;

params.name = "FullyConnected-name";

params.type = "InnerProduct";

opencv/modules/dnn/src/tensorflow/tf_importer.cpp

Line 1001 in 606c803

int id = dstNet.addLayer(name, "InnerProduct", layerParams);

opencv/modules/dnn/src/torch/torch_importer.cpp

Line 597 in 606c803

newModule->apiType = "InnerProduct";

WanliZhong · 2023-01-31T08:38:50Z

@alalek Why there are some warnings about gtest? What should I do to solve them?

alalek · 2023-01-31T08:48:57Z

Ignore. Warnings are not related to this PR - they are everywhere on "macOS - ARM64".

/cc @asmorkalov

alalek · 2023-01-31T09:00:39Z

Looks like macOS runner has been upgraded and enabled back(!) without any checks or necessary fixes.

last successful run: https://github.com/opencv/opencv/actions/runs/4041792474/jobs/6948734964
```
The CXX compiler identification is AppleClang 13.0.0.13000029
```
failed run: https://github.com/opencv/opencv/actions/runs/4051669640/jobs/6971463695
```
The CXX compiler identification is AppleClang 13.1.6.13160021
```

asmorkalov · 2023-01-31T09:32:45Z

@alalek My fault. Will take a look ASAP.

WanliZhong added category: dnn category: dnn (onnx) ONNX suport issues in DNN module labels Dec 29, 2022

WanliZhong mentioned this pull request Dec 29, 2022

Add test for GEMM with bias opencv/opencv_extra#1033

Merged

WanliZhong force-pushed the gemm_cuda branch from 9324635 to fa7935f Compare December 29, 2022 12:40

WanliZhong requested review from rogday and zihaomu December 29, 2022 12:53

WanliZhong marked this pull request as ready for review December 29, 2022 12:53

zihaomu approved these changes Jan 18, 2023

View reviewed changes

alalek reviewed Jan 19, 2023

View reviewed changes

make GEMM can be supported with transA and transB in CUDA

4718a4b

WanliZhong force-pushed the gemm_cuda branch from 44b8d69 to 4718a4b Compare January 31, 2023 07:15

alalek assigned zihaomu Feb 8, 2023

alalek merged commit 96a45e8 into opencv:4.x Feb 8, 2023

WanliZhong deleted the gemm_cuda branch May 16, 2023 12:33

asmorkalov mentioned this pull request May 31, 2023

(5.x) Merge 4.x #23718

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

DNN: make GEMM can be supported with transA and transB in CUDA#23061

DNN: make GEMM can be supported with transA and transB in CUDA#23061
alalek merged 1 commit intoopencv:4.xfrom
WanliZhong:gemm_cuda

WanliZhong commented Dec 29, 2022 •

edited by alalek

Loading

Uh oh!

zihaomu left a comment

Uh oh!

alalek Jan 19, 2023

Uh oh!

WanliZhong Jan 19, 2023 •

edited

Loading

Uh oh!

alalek Jan 31, 2023

Uh oh!

WanliZhong Feb 1, 2023 •

edited

Loading

Uh oh!

alalek Feb 1, 2023 •

edited by WanliZhong

Loading

Uh oh!

WanliZhong Feb 1, 2023

Uh oh!

WanliZhong Feb 8, 2023

Uh oh!

WanliZhong commented Jan 31, 2023

Uh oh!

alalek commented Jan 31, 2023

Uh oh!

alalek commented Jan 31, 2023

Uh oh!

asmorkalov commented Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	void ONNXImporter::parseMatMul(LayerParams& layerParams, const opencv_onnx::NodeProto& node_proto_)
	{
	opencv_onnx::NodeProto node_proto = node_proto_;
	CV_Assert(node_proto.input_size() == 2);
	layerParams.type = "InnerProduct";

	void ONNXImporter::parseGemm(LayerParams& layerParams, const opencv_onnx::NodeProto& node_proto)
	{
	CV_Assert(node_proto.input_size() >= 2);
	layerParams.type = "InnerProduct";

	case V1LayerParameter_LayerType_INNER_PRODUCT:
	return "InnerProduct";

	cv::dnn::LayerParams getParamFullyConnected(int output)
	{
	cv::dnn::LayerParams params;
	params.name = "FullyConnected-name";
	params.type = "InnerProduct";

Uh oh!

Conversation

WanliZhong commented Dec 29, 2022 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

zihaomu left a comment

Choose a reason for hiding this comment

Uh oh!

alalek Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

WanliZhong Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alalek Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

WanliZhong Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alalek Feb 1, 2023 • edited by WanliZhong Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WanliZhong Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

WanliZhong Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

WanliZhong commented Jan 31, 2023

Uh oh!

alalek commented Jan 31, 2023

Uh oh!

alalek commented Jan 31, 2023

Uh oh!

asmorkalov commented Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

WanliZhong commented Dec 29, 2022 •

edited by alalek

Loading

WanliZhong Jan 19, 2023 •

edited

Loading

WanliZhong Feb 1, 2023 •

edited

Loading

alalek Feb 1, 2023 •

edited by WanliZhong

Loading