-
-
Notifications
You must be signed in to change notification settings - Fork 16.6k
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
Parameterize max_det + inference default at 1000 #3215
Parameterize max_det + inference default at 1000 #3215
Conversation
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 @adrianholovaty, thank you for submitting a 🚀 PR! To allow your work to be integrated as seamlessly as possible, we advise you to:
- ✅ Verify your PR is up-to-date with origin/master. If your PR is behind origin/master an automatic GitHub actions rebase may be attempted by including the /rebase command in a comment body, 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 rebase 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
@adrianholovaty looks good, thanks for the PR. I've updated it for PEP8 and I've increased the inference default |
@adrianholovaty BTW I've increased the default as we've had a few issues/questions raised about this given the current 300 value default. Hopefully this will reduce instances of hitting the ceiling. For most common use cases (i.e. zidane.jpg and bus.jpg at --conf 0.25) the performance impact will be zero. |
@adrianholovaty PR is merged. Thank you for your contributions! |
@glenn-jocher Thanks! Glad to contribute back to this great project. :) |
@glenn-jocher If you increase max_det, is it then not also a good idea to increase max_nms? Maybe there also a need for increasing time_limit? |
@VdLMV no it's not necessary, and in fact the use case for large numbers of boxes into the torch NMS function has an unchanged max_det value of 300. |
* Added max_det parameters in various places * 120 character line * PEP8 * 120 character line * Update inference default to 1000 instances * Update inference default to 1000 instances Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com> (cherry picked from commit 3f74cd9)
* Added max_det parameters in various places * 120 character line * PEP8 * 120 character line * Update inference default to 1000 instances * Update inference default to 1000 instances Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com>
In this pull request, I've changed
max_det
(the maximum number of detections per image) so that it's more easily configurable. Previously it was always hard-coded to 300, and the only way to change it was to edit the Python code.Everything is backwards compatible — i.e., the default hasn't changed. Here's what's different:
utils.general.non_max_supression()
now takes amax_det
parameter.detect.py
now has a--max-det
command-line parameter, which lets you set the value.NMS
andAutoShape
classes inmodels/common.py
allow overridingmax_det
by setting the attribute on the class.I'm not sure whether the third thing (the
models/common.py
changes) is actually useful, as I don't know how that part of the code works. I sort of guessed based on the fact that the two classes already had some similar attributes (conf
,iou
,classes
).Finally, I should say it felt a bit dirty to copy the default
max_det=300
so that it's now duplicated in four separate places. I'd suggest putting that300
value in a single constant somewhere, then change all four places to use the constant instead of hard-coding300
. That way the value would only live in a single place. I'm happy to do this work; just let me know if there's an existing place for constants or whether I should create one.🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
Adjustment to the max number of detections in YOLOv5's NMS process.
📊 Key Changes
max_det
parameter (maximum detections per image) has been introduced tonon_max_suppression
function.max_det
value has been set to 1000, up from the previous hard-coded value of 300.detect.py
,common.py
, andgeneral.py
files are updated to pass and use themax_det
parameter.🎯 Purpose & Impact