Fix objectness is not assigned in dnn::region_layer by zhouzq-thu · Pull Request #22660 · opencv/opencv

zhouzq-thu · 2022-10-19T03:19:43Z

Fix objectness (dstData[index + 4]) is not assigned if new_coords == 1.

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

Fix objectness (dstData[index + 4]) is not assigned if new_coords == 1.

asmorkalov · 2022-10-19T06:40:27Z

@zhouzq-thu Could you point the issue, enabled model, or better to add test for the fix?

zhouzq-thu · 2022-10-19T06:51:47Z

@asmorkalov

In the latest 4.x branch, the objectness in the region_layer is only assigned at

opencv/modules/dnn/src/layers/region_layer.cpp

Line 279 in f378f02

dstData[index + 4] = logistic_activate(x); // logistic activation

But if YOLOv4 model is used (if new_coords == 1), dstData[index + 4] is not assigned. Therefore, some values in dstData are not predictable.

rogday · 2022-10-21T03:15:10Z

@AlexeyAB, could you validate please?

fengyuentau · 2022-10-31T02:59:49Z

modules/dnn/src/layers/region_layer.cpp

+                                dstData[box_index + 4] = srcData[p_index];

                                scale = srcData[p_index];
                                if (classfix == -1 && scale < thresh)


If I understand correctly, new_coords only have affects on how we calculate the width and height of bboxes. It does not relate to whether objectness should be kept or not. See these lines from darknet: https://github.com/AlexeyAB/darknet/blob/62e5549cef53bd11890808615e0bfae59cfd4491/src/yolo_layer.c#L132-L153

By they way, the scale in line 322 should be the objectness as you mentioned. Objectness is only used to calculate the final object score, and hence we do not need to keep it in the output.

The dstData[index + 4] is uninitialized if new_coords=1, but initialized if new_coords=0. I think it is not good.

If you have dstData[index + 4] values, you can optimize NMS by filtering some boxes (dstData[index + 4] < score_thresh), no need to find the maximum of all classes probabilities and check if it greater than score_thresh for each box.

ping @AlexeyAB

asmorkalov · 2023-04-11T13:43:54Z

@zhouzq-thu @fengyuentau Thanks for the patch and review. I made small investigation and outcome is the following:

Yolo models provides objectness score for all boxes and probability score per class. So, final box score is objectness*argmax(classes_score).
For Yolo v1-v3 new_coords==false and objectness is set in https://github.com/opencv/opencv/blob/4.x/modules/dnn/src/layers/region_layer.cpp#L279.
For Yolo v4+ new_coords==true and dstData[index + 4] is not set, but read in https://github.com/opencv/opencv/blob/4.x/modules/dnn/src/layers/region_layer.cpp#L321. It leads to unpredictable final score values. The issue may be amplified by NMS.

So, the proposed patch is relevant and correct.

fengyuentau · 2023-04-11T15:27:00Z

@zhouzq-thu @fengyuentau Thanks for the patch and review. I made small investigation and outcome is the following:

Yolo models provides objectness score for all boxes and probability score per class. So, final box score is objectness*argmax(classes_score).

For Yolo v1-v3 new_coords==false and objectness is set in https://github.com/opencv/opencv/blob/4.x/modules/dnn/src/layers/region_layer.cpp#L279.

For Yolo v4+ new_coords==true and dstData[index + 4] is not set, but read in https://github.com/opencv/opencv/blob/4.x/modules/dnn/src/layers/region_layer.cpp#L321. It leads to unpredictable final score values. The issue may be amplified by NMS.

So, the proposed patch is relevant and correct.

Thanks for the information. It is indeed should be set in case of initialization.

Just want to emphasize that the following do_nms_sort does not directly utilize the objectness score,

opencv/modules/dnn/src/layers/region_layer.cpp

Lines 357 to 390 in ebde9a5

    
           void do_nms_sort(float *detections, int total, float score_thresh, float nms_thresh) 
        
           { 
        
               std::vector<Rect2d> boxes(total); 
        
               std::vector<float> scores(total); 
        
               for (int i = 0; i < total; ++i) 
        
               { 
        
                   Rect2d &b = boxes[i]; 
        
                   int box_index = i * (classes + coords + 1); 
        
                   b.width = detections[box_index + 2]; 
        
                   b.height = detections[box_index + 3]; 
        
                   b.x = detections[box_index + 0] - b.width / 2; 
        
                   b.y = detections[box_index + 1] - b.height / 2; 
        
               } 
        
               std::vector<int> indices; 
        
               for (int k = 0; k < classes; ++k) 
        
               { 
        
                   for (int i = 0; i < total; ++i) 
        
                   { 
        
                       int box_index = i * (classes + coords + 1); 
        
                       int class_index = box_index + 5; 
        
                       scores[i] = detections[class_index + k]; 
        
                       detections[class_index + k] = 0; 
        
                   } 
        
                   NMSBoxes(boxes, scores, score_thresh, nms_thresh, indices); 
        
                   for (int i = 0, n = indices.size(); i < n; ++i) 
        
                   { 
        
                       int box_index = indices[i] * (classes + coords + 1); 
        
                       int class_index = box_index + 5; 
        
                       detections[class_index + k] = scores[indices[i]]; 
        
                   } 
        
               } 
        
           }

because the objectness score (scale in the following code piece) has been blended into the class score with class_score=objectness*argmax(classes_score):

opencv/modules/dnn/src/layers/region_layer.cpp

Lines 321 to 331 in ebde9a5

    
           scale = srcData[p_index]; 
        
           if (classfix == -1 && scale < thresh) 
        
           { 
        
               scale = 0;  // if(t0 < 0.5) t0 = 0; 
        
           } 
        
           int class_index = index_sample_offset + index * cell_size + 5; 
        
           for (int j = 0; j < classes; ++j) { 
        
               float prob = scale*srcData[class_index + j];  // prob = IoU(box, object) = t0 * class-probability 
        
               dstData[class_index + j] = (prob > thresh) ? prob : 0;  // if (IoU < threshold) IoU = 0; 
        
           }

fengyuentau

👍 Approve in case of initialization.

asmorkalov · 2023-04-12T06:34:14Z

I dig in tests and extra and found out that new_coords does not depend on Yolo version and we have test for YOLOv4x_mish with new_coords==1. For the case new_coords==1 objectness is taken from src tensor, but for another case it's taken from dst tensor. So on our side the final score is computed correctly, but exposed dst tensor contains random values for objectness. Further do_nms_sort does not use it and the issue is not visible with tests and popular use cases.

Update region_layer.cpp

c8561ea

Fix objectness (dstData[index + 4]) is not assigned if new_coords == 1.

asmorkalov requested a review from rogday October 19, 2022 06:39

asmorkalov assigned rogday Oct 19, 2022

asmorkalov added the category: dnn label Oct 19, 2022

asmorkalov added the pr: needs test New functionality requires minimal tests set label Oct 19, 2022

asmorkalov added this to the 4.7.0 milestone Oct 20, 2022

fengyuentau reviewed Oct 31, 2022

View reviewed changes

asmorkalov modified the milestones: 4.7.0, 4.8.0 Dec 12, 2022

asmorkalov requested review from asmorkalov and removed request for rogday March 13, 2023 13:26

asmorkalov self-assigned this Mar 13, 2023

fengyuentau approved these changes Apr 11, 2023

View reviewed changes

asmorkalov assigned fengyuentau and unassigned asmorkalov and rogday Apr 12, 2023

asmorkalov approved these changes Apr 12, 2023

View reviewed changes

asmorkalov merged commit 136121f into opencv:4.x Apr 12, 2023

asmorkalov added bug and removed pr: needs test New functionality requires minimal tests set labels Apr 12, 2023

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!

Fix objectness is not assigned in dnn::region_layer#22660

Fix objectness is not assigned in dnn::region_layer#22660
asmorkalov merged 1 commit intoopencv:4.xfrom
zhouzq-thu:4.x

zhouzq-thu commented Oct 19, 2022

Uh oh!

asmorkalov commented Oct 19, 2022

Uh oh!

zhouzq-thu commented Oct 19, 2022

Uh oh!

rogday commented Oct 21, 2022

Uh oh!

fengyuentau Oct 31, 2022

Uh oh!

zhouzq-thu Nov 1, 2022 •

edited

Loading

Uh oh!

asmorkalov commented Apr 11, 2023

Uh oh!

fengyuentau commented Apr 11, 2023

Uh oh!

fengyuentau left a comment

Uh oh!

asmorkalov commented Apr 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

zhouzq-thu commented Oct 19, 2022

Pull Request Readiness Checklist

Uh oh!

asmorkalov commented Oct 19, 2022

Uh oh!

zhouzq-thu commented Oct 19, 2022

Uh oh!

rogday commented Oct 21, 2022

Uh oh!

fengyuentau Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

zhouzq-thu Nov 1, 2022 • 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!

asmorkalov commented Apr 11, 2023

Uh oh!

fengyuentau commented Apr 11, 2023

Uh oh!

fengyuentau left a comment

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented Apr 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zhouzq-thu Nov 1, 2022 •

edited

Loading