-
-
Notifications
You must be signed in to change notification settings - Fork 16.2k
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
DetectMultiBackend() default stride=32
#7342
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 @rglkt, 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
@@ -296,7 +296,7 @@ def __init__(self, weights='yolov5s.pt', device=torch.device('cpu'), dnn=False, | |||
super().__init__() | |||
w = str(weights[0] if isinstance(weights, list) else weights) | |||
pt, jit, onnx, xml, engine, coreml, saved_model, pb, tflite, edgetpu, tfjs = self.model_type(w) # get backend | |||
stride, names = 64, [f'class{i}' for i in range(1000)] # assign defaults | |||
stride, names = 32, [f'class{i}' for i in range(1000)] # assign defaults |
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.
Seems the P6 series need stride 64, so it would be better to set this parameter optional here.
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.
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.
sorryοΌi knowοΌ set it on cmd argumentοΌ i try it
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.
P6 is yolov5n6, yolov5s6, yolov5m6 and yolov5l6, introduced from tag 5.0.
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.
thanksοΌand i have added argument --stride
for more information, see https://pre-commit.ci
@rglkt some model formats (like PyTorch) can access attached metadata (like stride) to dynamically select the minimum viable stride, while other formats (like ONNX) do not have any metadata and thus lack stride information, in which case stride 64 is assumed since it is valid for both P5 models (minimum stride 32) and P6 models (minimum stride 64). The only alternative here is to run an image and compare the input to the output to empirically attempt a stride determination at inference time, which would add delay on DetectMultiBackend init. I'll think this over a bit and see what we can do here. |
@rglkt I've implemented ONNX metadata saving and reading now which resolves any stride issues for ONNX models. Unfortunately the rest of the formats are not resolved for this issue, like the engine issue in #7164. One limitation in this PR is that detect.py is only one entrypoint for inference, we also have val.py and PyTorch Hub model inference. I don't see a good solution to this other than to try to keep attaching custom metadata to each export format. So far I have PyTorch, TorchScript, and now ONNX available with |
Thank you for your work, I am a senior who is doing graduation design. This code helped me a lot, I think my pr is not the best solution, but I want to report the problem that other people will also encounter in the code and hope to contribute some power to the code. Thanks again for the open source code |
stride=32
@rglkt PR is merged. Thank you for your contributions to YOLOv5 π and Vision AI β I think is probably a slight improvement over master, as now errors will appear on incorrect The problem's not solved of course, as now errors will appear when users with P6 exported models attempt inference on stride 32 images, but at least the error will appear appropriately on an incorrect argument value. |
* set common default stride as 32 * restore default stride, and set it on argument optional * fix wrong use of opt * fix missing parameter of stride * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix format of parameters * Update val.py * Update common.py Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com>
π οΈ PR Summary
Made with β€οΈ by Ultralytics Actions
π Summary
Optimization of default stride setting for YOLOv5 models.
π Key Changes
π― Purpose & Impact