-
Notifications
You must be signed in to change notification settings - Fork 617
[Example] Yolo12 Detection sample with OpenVINO/XNNPACK backend #10156
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/10156
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit 9e1d95f with merge base f0a7d10 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
FYI On the mlock error, you can try switching the load mode in the Module constructor (or data loader) to Mmap (from Mlock). IMO that should be the default. |
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.
I think some sanity checking needs to be added to this example.
@@ -88,7 +88,7 @@ def set_ignored_scope( | |||
names: Optional[List[str]] = None, | |||
patterns: Optional[List[str]] = None, | |||
types: Optional[List[str]] = None, | |||
subgraphs: Optional[List[Tuple[List[str], List[str]]]] = None, | |||
subgraphs: Optional[List[nncf.Subgraph]] = None, |
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.
Could you manage subgraphs
without changing the function signature?
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.
Done
This helped, thanks! |
4c302d6
to
60d9cfb
Compare
@lucylq, @jackzhxng, could you please take a look? |
@@ -0,0 +1,198 @@ | |||
#!/bin/bash |
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.
Does this need to be running in CI?
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.
This is copy-paste from the other similar .sh files like this https://github.com/pytorch/executorch/blob/main/.ci/scripts/test_openvino.sh
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.
No my question is this script is not actually running in ci. If so why is it inside .ci folder
backends/openvino/README.md
Outdated
git clone https://github.com/daniil-lyakhov/openvino.git | ||
cd openvino && git checkout dl/executorch/yolo12 |
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.
is this meant to be left like this?
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.
We are working on a fix in the openvino itegration, I'll update the branch as soon as we will have it merged
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.
Updated
@@ -0,0 +1,168 @@ | |||
#include "inference.h" |
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.
@larryliu0820 have we setup examples repo already? I am wondeirng if we should move it there.
@@ -0,0 +1,151 @@ | |||
#ifndef INFERENCE_H |
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.
nit: maybe orgnaize folders into export and inference
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.
Left some comments. Didnt review in details since this is largely in examples folder. At high level can you include some performance numbers in the summary
@guangy10 to see if this can be done also via optimum-et |
HF Transformers seem to have a different one |
b3e69c5
to
794cf64
Compare
Please take a look https://github.com/pytorch/executorch/pull/10156/files#diff-a2182fd228e1b70702adceb8eacfcb42462b65dab17a66113a43ba511ee6988aR15-R22 |
794cf64
to
546c939
Compare
WIll take a look. Please do ping in case this slips through. I apologize that this has been waiting for so long |
from torch.ao.quantization.quantize_pt2e import convert_pt2e, prepare_pt2e | ||
from torch.export.exported_program import ExportedProgram | ||
from torch.fx.passes.graph_drawer import FxGraphDrawer | ||
from ultralytics import YOLO |
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.
is this different from hf yolo?
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.
Could you please share a link to the HF yolo? I believe that Ultralitics is the primary source of the Yolo12 models
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.
Aah. I see. Yeah while looking for yolo12 + hf I am also coming across mentions of ultralitics.
I was trying to find the source code of it. If you can point me to that, it would be great. I am trying to understand how bounding box related logic is handled in the model and its exportability
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.
The ultralytics Yolo repo is quite complex due to the fact that a single class (YOLO) is handling all available yolo versions + training/evaluation/inference code as well.
The inference pipeline could be found here
https://github.com/ultralytics/ultralytics/blob/main/ultralytics/engine/validator.py#L202-L226
With preprocessing https://github.com/ultralytics/ultralytics/blob/main/ultralytics/models/yolo/detect/val.py#L66-L81
and postprocessing
https://github.com/ultralytics/ultralytics/blob/main/ultralytics/models/yolo/detect/val.py#L113-L133
The example code is based on the YOLOv8-CPP-Inference example (the bounding box logic could be found here https://github.com/ultralytics/ultralytics/blob/main/examples/YOLOv8-CPP-Inference/inference.cpp#L16)
Summarizing my work, I've replaced the ONNX
backend with the ExecuTorch and made sure everything is working
backends/openvino/README.md
Outdated
@@ -45,8 +45,9 @@ Before you begin, ensure you have openvino installed and configured on your syst | |||
### Build OpenVINO from Source | |||
|
|||
```bash | |||
git clone https://github.com/openvinotoolkit/openvino.git | |||
cd openvino && git checkout releases/2025/1 | |||
git clone https://github.com/daniil-lyakhov/openvino.git |
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.
Lets wait till the fix is landed? I prefer that we dont have to do this
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.
Fix is finally landed on the master branch! The PR is updated
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.
lets wait till the fix is landed so that we dont have to patch to your personal repo + branch
The fix is finally merged to the openvino master! The README and the test script are updated accordingly |
Thank you. Accepted. Will wait for rest of the CI and merge |
Just realized, i didnt approve the workflows. Just did |
Linters and some other jobs were failed most likely due to the old base commit. Rebased the PR and fixed the linter |
kicked off the run |
some tests are failing. not sure if they are realted |
Looks like some sporicidal problems with the XNNPACKQuantizer
and a problem with access to a json file for the llama tests on a mac os
Most likely this commits will fix the problems #12096 #12089. Merging the main branch to my PR |
Unrelated failures. merging |
…nd" (#12136) Reverts #10156 @daniil-lyakhov I'm reverting this PR because the gif file is large. Can you please remove the gif file and re-land?
…rch#10156) ### Summary Ultralytics Yolo12 Detection sample to expand example models with the detection task: 1) Export script to get executorch yolo model lowered to OpenVINO/XNNPACK 2) Demo app which gets a model and a video as an input and outputs an annotated video Changes in the OpenVINO backend: * OpenVINO repo branch is updated to support yolo12 models (Commit to Openvino master WIP) * Minor fixes in quantizer.py ### Test plan #### Warning: OpenCV should be installed in the environment. OpenVINO: ``` ./.ci/scripts/test_yolo12.sh -model yolo12n -mode openvino -pt2e_quantize OFF -upload tmp_ov_run -video_path path/to/mp4/file ``` XNNPACK: ``` ./.ci/scripts/test_yolo12.sh -model yolo12n -mode xnnpack -pt2e_quantize OFF -upload tmp_xnnpack_run -video_path path/to/mp4/file ``` ### Issues: * Quantization does not work in both backends, issues WIP. Marked by NonImplemented error by now * OpenVINO is being build twice because it is not present in the [executorch-config.cmake](https://github.com/pytorch/executorch/blob/main/tools/cmake/executorch-config.cmake) file. Perhaps we could add the OpenVINO backend to the config file? CC: @ynimmaga @suryasidd @alexsu52
### Summary Follow up #12136 Revert of the revert #10156 The big gif is removed from the PR CC: @kimishpatel
…rch#10156) ### Summary Ultralytics Yolo12 Detection sample to expand example models with the detection task: 1) Export script to get executorch yolo model lowered to OpenVINO/XNNPACK 2) Demo app which gets a model and a video as an input and outputs an annotated video Changes in the OpenVINO backend: * OpenVINO repo branch is updated to support yolo12 models (Commit to Openvino master WIP) * Minor fixes in quantizer.py ### Test plan #### Warning: OpenCV should be installed in the environment. OpenVINO: ``` ./.ci/scripts/test_yolo12.sh -model yolo12n -mode openvino -pt2e_quantize OFF -upload tmp_ov_run -video_path path/to/mp4/file ``` XNNPACK: ``` ./.ci/scripts/test_yolo12.sh -model yolo12n -mode xnnpack -pt2e_quantize OFF -upload tmp_xnnpack_run -video_path path/to/mp4/file ``` ### Issues: * Quantization does not work in both backends, issues WIP. Marked by NonImplemented error by now * OpenVINO is being build twice because it is not present in the [executorch-config.cmake](https://github.com/pytorch/executorch/blob/main/tools/cmake/executorch-config.cmake) file. Perhaps we could add the OpenVINO backend to the config file? CC: @ynimmaga @suryasidd @alexsu52
…nd" (pytorch#12136) Reverts pytorch#10156 @daniil-lyakhov I'm reverting this PR because the gif file is large. Can you please remove the gif file and re-land?
### Summary Follow up pytorch#12136 Revert of the revert pytorch#10156 The big gif is removed from the PR CC: @kimishpatel
Summary
Ultralytics Yolo12 Detection sample to expand example models with the detection task:
Changes in the OpenVINO backend:
Test plan
Warning:
OpenCV should be installed in the environment.
OpenVINO:
XNNPACK:
Issues:
CC: @ynimmaga @suryasidd @alexsu52