-
-
Notifications
You must be signed in to change notification settings - Fork 16.3k
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 confidence threshold for ClearML debug images #9174
Conversation
The confidence is converted to a percentage on line 144, but it is being compared to a default conf_threshold value of a decimal value instead of percent value. Signed-off-by: HighMans <42877729+HighMans@users.noreply.github.com>
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 @HighMans, 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 upstream/master. If your PR is behind upstream/master an automatic GitHub Actions merge may be attempted by writing /rebase in a new comment, or by running the following code, replacing 'feature' with the name of your local branch:
git remote add upstream https://github.com/ultralytics/yolov5.git
git fetch upstream
# git checkout feature # <--- replace 'feature' with local branch name
git merge upstream/master
git push -u origin -f
- ✅ Verify all 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
Thanks for the PR! yolov5/utils/loggers/clearml/clearml_utils.py Line 144 in cff9717
|
The confidence is converted to a percentage here: yolov5/utils/loggers/clearml/clearml_utils.py Line 144 in cff9717
But then it's being compared two lines after here to conf_threshold which by default is .25 and not 25%: yolov5/utils/loggers/clearml/clearml_utils.py Line 147 in cff9717
yolov5/utils/loggers/clearml/clearml_utils.py Line 125 in cff9717
|
Yup. Makes sense |
Hey nice catch! That was sloppy coding from my part. That said, don't you think it makes more sense to convert the percentage to decimal and keep the threshold like it is? I think this is pretty much default when passing along a threshold right? That it is between 0 and 1? If you agree, can you leave the argument as it is, but just set the confidence to a decimal instead of a percentage? Then inside the f-string we can multiply it by 100 for visualisation, but it would have no effect on the original parameter which can then be properly compared to the threshold :) |
This reverts commit f84a099.
@thepycoder yes I think it makes most sense to just keep the decimal and not convert it to percentage. |
Fix the confidence percentage is being compared to a decimal value.
That makes a lot more sense, should be fixed now. yolov5/utils/loggers/clearml/clearml_utils.py Lines 143 to 149 in 6d40357
|
Lgtm! Thank a lot :) |
@HighMans @thepycoder @AyushExel PR is merged. Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐ |
* Fix confidence threshold The confidence is converted to a percentage on line 144, but it is being compared to a default conf_threshold value of a decimal value instead of percent value. Signed-off-by: HighMans <42877729+HighMans@users.noreply.github.com> * Revert "Fix confidence threshold" This reverts commit f84a099. * Fix confidence comparison Fix the confidence percentage is being compared to a decimal value. Signed-off-by: HighMans <42877729+HighMans@users.noreply.github.com>
The confidence is converted to a percentage on line 144, but it is being compared to a default conf_threshold value of a decimal value instead of percent value.
Signed-off-by: HighMans 42877729+HighMans@users.noreply.github.com
🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
Improved variable naming and fixed confidence threshold logic in ClearML image logging.
📊 Key Changes
confidence
variable toconfidence_percentage
for better clarity.conf
) instead of the percentage.🎯 Purpose & Impact