Skip to content

Update op_conv.cpp#22419

Closed
scottchou007 wants to merge 1 commit intoopencv:4.xfrom
scottchou007:patch-1
Closed

Update op_conv.cpp#22419
scottchou007 wants to merge 1 commit intoopencv:4.xfrom
scottchou007:patch-1

Conversation

@scottchou007
Copy link
Copy Markdown
Contributor

@scottchou007 scottchou007 commented Aug 23, 2022 •
edited
Loading

Leave bias tensor uninitialized causes issues in parallel convolution kernel "conv48" which always uses bias as input.
This change proposes to pass a zero tensor if no bias is used for to latter buffer binding, no matter if kernel uses or not for safety.

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

@scottchou007
Update op_conv.cpp
a77819d 
Leave bias tensor uninitialized causes issues in parallel convulsion "conv48".
This change propose to pass a zero tensor if no bias is used for to latter buffer binding, no matter if shader uses or not for safety.
@asmorkalov asmorkalov requested review from rogday and zihaomu August 24, 2022 07:20
@asmorkalov asmorkalov added category: dnn pr: needs test New functionality requires minimal tests set labels Aug 24, 2022
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Aug 24, 2022

@scottchou007 Could you add some test case or at least point to the network that causes the issue?

@scottchou007
Copy link
Copy Markdown
Contributor Author

scottchou007 commented Aug 24, 2022

Add test case descriptions.
The issue was found in several networks in original Opencv case "opencv_test_dnn" on NXP iMX8 ARM64 platform with Vulkan backend. It happens randomly from the networks that uses parallel convolution kernel.
Here is part of the error log output from the network "DNNTestNetwork.MobileNet_SSD_v1_TensorFlow":

First run
Unmatched reference: class 10 score 0.846946 box [0.0205273 x 0.0676169 from (0.385021, 0.319737)] IoU diff: 1
Expected: (refScores[i]) <= (confThreshold), actual: 0.846946 vs 0.2
First run
Unmatched reference: class 1 score 0.798083 box [0.0640923 x 0.260031 from (0.196823, 0.36867)] IoU diff: 1
Expected: (refScores[i]) <= (confThreshold), actual: 0.798083 vs 0.2
First run
Unmatched reference: class 3 score 0.691368 box [0.0392624 x 0.0452221 from (0.450937, 0.470303)] IoU diff: 1
Expected: (refScores[i]) <= (confThreshold), actual: 0.691368 vs 0.2
First run
Unmatched reference: class 1 score 0.269372 box [0.070512 x 0.268825 from (0.175035, 0.368568)] IoU diff: 1
Expected: (refScores[i]) <= (confThreshold), actual: 0.269372 vs 0.2
First run

@fengyuentau fengyuentau self-requested a review August 25, 2022 01:28
@fengyuentau
Copy link
Copy Markdown
Member

fengyuentau commented Sep 4, 2022

Most of the errors are addressed (markded as red) with this pull request, but it brings a new failed tesst (marked as green):

+ [  FAILED  ] DNNTestNetwork.FastNeuralStyle_eccv16/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] DNNTestNetwork.ResNet_50/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] DNNTestNetwork.ENet/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] DNNTestNetwork.MobileNet_SSD_v1_TensorFlow/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] DNNTestNetwork.MobileNet_SSD_v1_TensorFlow_Different_Width_Height/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] DNNTestNetwork.MobileNet_SSD_v2_TensorFlow/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] DNNTestNetwork.opencv_face_detector/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] DNNTestNetwork.DenseNet_121/0, where GetParam() = VKCOM/VULKAN
[  FAILED  ] Test_Caffe_nets.Colorization/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Caffe_nets.DenseNet_121/0, where GetParam() = VKCOM/VULKAN
[  FAILED  ] Test_Caffe_nets.FasterRCNN_vgg16/0, where GetParam() = VKCOM/VULKAN
[  FAILED  ] Test_Caffe_nets.FasterRCNN_zf/0, where GetParam() = VKCOM/VULKAN
[  FAILED  ] Test_Caffe_nets.RFCN/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Darknet_nets.YoloVoc/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Darknet_nets.TinyYoloVoc/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Darknet_nets.YOLOv3/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Darknet_nets.YOLOv4/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Darknet_nets.YOLOv4_tiny/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Darknet_nets.YOLOv4x_mish/0, where GetParam() = VKCOM/VULKAN
[  FAILED  ] Test_Caffe_layers.Softmax/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Model.DetectRegion/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Model.DetectRegionWithNmsAcrossClasses/0, where GetParam() = VKCOM/VULKAN
[  FAILED  ] Test_Model.DetectionOutput/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Model.Keypoints_pose/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Model.TextDetectionByDB/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Model.TextDetectionByEAST/0, where GetParam() = VKCOM/VULKAN
[  FAILED  ] Test_ONNX_conformance.Layer_Test/test_div_bcast_VKCOM_VULKAN, where GetParam() = (test_div_bcast, VKCOM/VULKAN)
[  FAILED  ] Test_ONNX_conformance.Layer_Test/test_mul_bcast_VKCOM_VULKAN, where GetParam() = (test_mul_bcast, VKCOM/VULKAN)
- [  FAILED  ] Test_ONNX_layers.Squeeze/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_ONNX_layers.Quantized_Gemm/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_ONNX_layers.Quantized_Gemm/1, where GetParam() = OCV/CPU
- [  FAILED  ] Test_ONNX_nets.TinyYolov2/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_ONNX_nets.LResNet100E_IR/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_ONNX_nets.Emotion_ferplus/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_TensorFlow_nets.MobileNet_v1_SSD/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_TensorFlow_nets.opencv_face_detector_uint8/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_TensorFlow_nets.EAST_text_detection/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_TensorFlow_nets.Mask_RCNN/0, where GetParam() = VKCOM/VULKAN
[  FAILED  ] Test_TensorFlow_nets.EfficientDet/0, where GetParam() = VKCOM/VULKAN
- [  FAILED  ] Test_Torch_nets.ENet_accuracy/0, where GetParam() = VKCOM/VULKAN

@scottchou007
Copy link
Copy Markdown
Contributor Author

scottchou007 commented Sep 6, 2022 •
edited
Loading

created a new PR with updated change for the same issue.
#22479

Change in this PR will be abandoned.
.

@scottchou007 scottchou007 marked this pull request as draft September 6, 2022 08:00
@scottchou007 scottchou007 mentioned this pull request Sep 6, 2022
Merged
6 tasks
@asmorkalov asmorkalov closed this Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rogday rogday Awaiting requested review from rogday

@zihaomu zihaomu Awaiting requested review from zihaomu

@fengyuentau fengyuentau Awaiting requested review from fengyuentau

Assignees

No one assigned

Labels

category: dnn pr: needs test New functionality requires minimal tests set

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@scottchou007 @asmorkalov @fengyuentau