-
-
Notifications
You must be signed in to change notification settings - Fork 16.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix tick labels for background FN/FP #9414
Fix tick labels for background FN/FP #9414
Conversation
In the confusion matrix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋 Hello @hotohoto, thank you for submitting a YOLOv5 🚀 PR! To allow your work to be integrated as seamlessly as possible, we advise you to:
- ✅ Verify your PR is up-to-date with
ultralytics/yolov5
master
branch. If your PR is behind you can update your code by clicking the 'Update branch' button or by runninggit pull
andgit merge master
locally. - ✅ Verify all YOLOv5 Continuous Integration (CI) checks are passing.
- ✅ Reduce changes to the absolute minimum required for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." — Bruce Lee
@hotohoto confusion matrix labels are correct. Backgrounds that are predicted as a class are a False Negative (FN) and vice versa. |
I mean it. That's the reason why I think the labels are incorrect. 🤔 Concretely, the last column in a confusion matrix is the case when it's predicted as a class but the ground truth is background. So it's a False Negative like you said. But it's labeled as Fasle Positive currently which is incorrect. And vice versa. (You may refer to the image at #8000 (comment)) |
@hotohoto the last column indicates cells that were identified as a class but in reality contains nothing. This is the definition of a FP detection, which is why the label shows 'background FP' But probably to simplify things I'll just remove the FP and TP from the matrix and just leave 'background' as all the other classes. |
Thanks. @glenn-jocher |
Signed-off-by: Glenn Jocher <glenn.jocher@ultralytics.com>
@hotohoto PR is merged. Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐ |
@glenn-jocher @hotohoto Hi,I'm confused by these two reply. Are they all talking about the same situation as shown in the follow figure, that is, backgrounds(in reality contains nothing) are predicted as a class? |
I also have a question about the following Lines 175 to 178 in a83d2a5
When all detections are False Positive background, which means all values of the iou will be less than iou_thres .And n must be False in this case,causing none of the backbound FP is counted into self.matrix .I think it should be modified as follows:
|
@avril-wang1214 Thank you for the feedback. I'll review the provided information and make the necessary adjustments to the code. Your insights are greatly appreciated! |
Fix tick labels for background FN/FP in the confusion matrix.
They needs to be switched to each other.
This issue has been raised within #8000.
🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
Improved clarity in confusion matrix terminology.
📊 Key Changes
ticklabels
variable.ticklabels
reflect the updated terminology and apply to both x and y axes.🎯 Purpose & Impact