Bug in the process_batch function in val.py

[kimberlykang]

Search before asking

• I have searched the YOLOv5 issues and found no similar bug report.

YOLOv5 Component

No response

Bug

In the process_batch function, detections are filtered by IoU and detected class. This filtered detections are named matches and the following procedures are performed

1. sort matches by IoU.
2. leave only unique detection indices
3. leave only unique label indices.

The expected outcome of the above is to leave unique detection results with high IoU. However in some cases, results with lower IoU are not filtered out and become the final result, because of the lack of sort between 2 and 3

There is not sort because matches are already filtered by IoU threshold and the calculation of AP does not care about IoU?

Environment

No response

Minimal Reproducible Example

# The initial matches
# array([[          1,           0,     0.51507],
#        [          1,           3,     0.78271], 
#        [          1,          11,     0.51642],
#        [          1,          12,     0.54807]], dtype=float32)

matches = matches[matches[:, 2].argsort()[::-1]]   # 1. sort `matches` by IoU.
# array([[          1,           3,     0.78271],
#        [          1,          12,     0.54807],
#        [          1,          11,     0.51642],
#        [          1,           0,     0.51507]], dtype=float32)

matches = matches[np.unique(matches[:, 1], return_index=True)[1]]   # 2. leave only unique detection indices
# array([[          1,           0,     0.51507],
#        [          1,           3,     0.78271],
#        [          1,          11,     0.51642],
#        [          1,          12,     0.54807]], dtype=float32)

# matches = matches[matches[:, 2].argsort()[::-1]]   # sort operation is commented out

matches = matches[np.unique(matches[:, 0], return_index=True)[1]]   #  3. leave only unique label indices.
# array([[          1,           0,     0.51507]], dtype=float32)

Additional

No response

Are you willing to submit a PR?

• Yes I'd like to help by submitting a PR!

[glenn-jocher]

@kimberlykang thank you for your report and valuable contribution to YOLOv5. You are correct that the matches are sorted only by IoU, and then filtered by label. While it is expected that the label filtering would remove low IoU matches, there may be cases where the same label is used for detections with different IoU scores. Therefore, we will modify the process_batch function to first sort by IoU score, then by label, to ensure that only the highest IoU detection for each label is kept. We appreciate your willingness to submit a PR and will review it promptly. Please let us know if you have any further questions or suggestions.

[kimberlykang]

@glenn-jocher I think what the process_batch function need is another sort by IoU.

matches = matches[matches[:, 2].argsort()[::-1]]                   # line 1
matches = matches[np.unique(matches[:, 1], return_index=True)[1]]  # line 2
# matches = matches[matches[:, 2].argsort()[::-1]]                 # line 3
matches = matches[np.unique(matches[:, 0], return_index=True)[1]]  # line 4

From the above code in line 1, matches are sorted by IoU. However, in line 2, matches become unsorted because of the np.unique.

In line 2, matches[:, 1] is the detection indices which have higher IoU than the threshold and have the correct class. Then this detection indices are filtered so that there are only unique indices by np.unique function. Inside the np.unique, _unique1d are called and it starts filtering indices by sorting the detection them, which does not have IoU information. Below is the code snippet of the _unique1d.

    ar = np.asanyarray(ar).flatten()

    optional_indices = return_index or return_inverse

    if optional_indices:
        perm = ar.argsort(kind='mergesort' if return_index else 'quicksort')
        aux = ar[perm]

Finally np.unique returns the unique indices, but not in the original order, but in the new order resulted from the indices sort inside the _unique1d

Due to this reason above, I think what it needs is to just uncomment the line 3 so that after np.unique operation there is another sort by IoU. If the indices are sorted and go through np.unique operation, the first occurred unique index is kept, which has the highest IoU.

[github-actions]

👋 Hello there! We wanted to give you a friendly reminder that this issue has not had any recent activity and may be closed soon, but don't worry - you can always reopen it if needed. If you still have any questions or concerns, please feel free to let us know how we can help.

For additional resources and information, please see the links below:

• Docs: https://docs.ultralytics.com
• HUB: https://hub.ultralytics.com
• Community: https://community.ultralytics.com

Feel free to inform us of any other issues you discover or feature requests that come to mind in the future. Pull Requests (PRs) are also always welcomed!

Thank you for your contributions to YOLO 🚀 and Vision AI ⭐

[glenn-jocher]

@kimberlykang Thank you for the detailed explanation. Your insights are valuable. We have taken note of your suggestion, and we agree that adding another sort by IoU after the np.unique operation, as you described, is indeed the proper approach to ensure that only the detections with the highest IoU are kept. We will incorporate this modification in the upcoming release and acknowledge your contribution in the release notes. Your understanding of the inner workings of the process_batch function is commendable, and we appreciate your effort in improving YOLOv5. If you have any further suggestions or questions, please feel free to share.

[Love-syntacticSugar]

@glenn-jocher
Hello, author. I also noticed a similar situation in the YOLOv8 source code. In the process_batch method in the metrics.py file, the following code is indeed used:

matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 1], return_index=True)[1]]
matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 0], return_index=True)[1]]

However! In the match_predictions method in the validator.py file, you are still using:

matches = matches[iou[matches[:, 0], matches[:, 1]].argsort()[::-1]]
matches = matches[np.unique(matches[:, 1], return_index=True)[1]]
matches = matches[np.unique(matches[:, 0], return_index=True)[1]]

Is this an oversight on your part?

[glenn-jocher]

Hello! Thanks for pointing this out. It seems like the match_predictions method in validator.py could indeed benefit from an additional sort by IoU after the np.unique operation on detection indices, similar to what is done in process_batch. This would ensure consistency across methods and that the highest IoU detections are retained. We'll review this and consider updating it in a future release. Your keen observation is much appreciated! 🌟 If you have any more insights or questions, feel free to share.

[Tim-Agro]

Hello @glenn-jocher! I have a similar question to @Love-syntacticSugar

In the YOLOv5 source code, within metrics.py, the matches matrix was calculated in the ConfusionMatrix.process_batch method using the following lines:

matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 1], return_index=True)[1]]
matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 0], return_index=True)[1]]

However, in val.py, the process_batch method is implemented with the third line commented out:

matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 1], return_index=True)[1]]
# matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 0], return_index=True)[1]]

If these two parts should be keep the same?

[glenn-jocher]

Hello @Tim-Agro,

Thank you for your detailed observation and for bringing this to our attention!

To address your question, the process_batch method in val.py and the corresponding logic in metrics.py should indeed be consistent to ensure the highest IoU detections are retained. The commented-out line in val.py appears to be an oversight and should be uncommented to match the logic in metrics.py.

Here's the corrected code for val.py:

matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 1], return_index=True)[1]]
matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 0], return_index=True)[1]]

This ensures that after filtering unique detection indices, the matches are sorted again by IoU before filtering unique label indices, maintaining the highest IoU detections.

If you haven't already, please verify that this issue persists with the latest versions of torch and the YOLOv5 repository. Keeping your environment up-to-date can sometimes resolve unexpected issues.

If you encounter any further issues or have additional questions, please don't hesitate to ask. Your contributions and insights are invaluable to the YOLO community and the Ultralytics team! 😊

[Tim-Agro]

Hi @glenn-jocher, thank you for your comments.

Upon thoroughly reviewing the purpose of these four lines and considering the answer @Terry-Zheng from issue #12543, I discovered that line 3 should be commented out. This is because a high IOU does not indicate a high confidence value.

hi there! Thanks for reaching out. The line you mentioned was commented out intentionally to address a specific use case. We continually revise and validate the YOLOv5 codebase to ensure optimal performance across various scenarios. Your feedback is valuable for ongoing improvements.

@glenn-jocher Thanks! You're right, line 92 should be commented, the higher IoU doesn't necessarily mean the higher confidence. In the process_batchmethod, after executing lines 90 and 91, each detection has found its highest IoU GT box, and then higher confidence, instead of higher IoU is the standard for GT boxes to select their best matches detection. Actually, detections are sorted in descending confidence order after NMS, so each GT box chooses detections with lower id. After line 91 is executed, the detections are sorted in ascending id order(descending confidence), so there's no need to perform another argsort, which will disrupt the original confidence order.

Examples

I did some validation on coco128(with batch128 and imgsize640), for image(train2017/000000000431.jpg) in the process_batch method, with IoU(0.5) in the first loop:

matches = torch.cat((torch.stack(x, 1), iou[x[0], x[1]][:, None]), 1).cpu().numpy()# [label, detect, iou]

matches = matches[matches[:, 2].argsort()[::-1]] #line90
# array([[          1,           0,     0.93549],
#        [          2,          15,      0.7387],
#        [          2,           1,     0.71555],
#        [          0,           2,      0.6466],
#        [          2,          17,     0.57293]], dtype=float32)

matches = matches[np.unique(matches[:, 1], return_index=True)[1]] #line91
# array([[          1,           0,     0.93549],
#        [          2,           1,     0.71555],
#        [          0,           2,      0.6466],
#        [          2,          15,      0.7387],
#        [          2,          17,     0.57293]], dtype=float32)

# matches = matches[matches[:, 2].argsort()[::-1]] #line92
matches = matches[np.unique(matches[:, 0], return_index=True)[1]] #line93
# array([[          0,           2,      0.6466],
#        [          1,           0,     0.93549],
#        [          2,           1,     0.71555]], dtype=float32)

We can find that (GT2, detect1) is in front of (GT2, detect15) after line 91, although the latter has higher iou, but we can get the confidence of detect1 and 15 by the following codes:

detections[[1,15], 4]
# tensor([0.77463, 0.00325], device='cuda:0')

So we can see that detect1 has much higher confidence than detect15. If we uncomment line 92, it will make us choose detect15 as the match of GT2.

Therefore, to ensure consistency in between val.py and metrics.py, line 3 should be commented out in both files.

matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 1], return_index=True)[1]]
# matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 0], return_index=True)[1]]

[glenn-jocher]

Hi @Tim-Agro,

Thank you for your detailed analysis and for referencing the discussion from issue #12543. Your insights are very much appreciated!

You are correct that a high IoU does not necessarily correlate with high confidence. The intention behind commenting out the third line is to ensure that detections are matched based on confidence rather than IoU after the initial sorting and unique operations. This approach aligns with the logic that detections are sorted in descending order of confidence after Non-Maximum Suppression (NMS), ensuring that each ground truth box selects the detection with the highest confidence.

To maintain consistency between val.py and metrics.py, we should indeed ensure that the third line remains commented out in both files. This will help avoid any disruption in the confidence order of detections.

Here is the consistent code snippet for both files:

matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 1], return_index=True)[1]]
# matches = matches[matches[:, 2].argsort()[::-1]]
matches = matches[np.unique(matches[:, 0], return_index=True)[1]]

We encourage you to verify that this issue persists with the latest versions of torch and the YOLOv5 repository. Keeping your environment up-to-date can sometimes resolve unexpected issues.

Thank you once again for your valuable feedback and for helping improve YOLOv5. If you have any further questions or suggestions, please feel free to share. Your contributions are invaluable to the YOLO community and the Ultralytics team! 😊