[Examples] Yolo26 Sample for OpenVINO/XNNPACK#18583
[Examples] Yolo26 Sample for OpenVINO/XNNPACK#18583daniil-lyakhov wants to merge 4 commits intopytorch:mainfrom
Conversation
daniil-lyakhov
requested review from
kirklandsign,
larryliu0820 and
lucylq
as code owners
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18583
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 1 Cancelled Job, 3 Unrelated FailuresAs of commit 5696739 with merge base 4e2ae9c ( NEW FAILURES - The following jobs have failed:
CANCELLED JOB - The following job was cancelled. Please retry:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
bot
added
the
CLA Signed
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
Updates the ExecuTorch YOLO example to use the Ultralytics YOLO26 family (replacing YOLO12), including C++ demo code, export/validation tooling, and CI coverage for OpenVINO/XNNPACK.
Changes:
- Add/refresh YOLO26 example (C++ runner, export+validate script, docs, CMake target).
- Update OpenVINO quantizer ignored-scope handling to support subgraph tuples.
- Update CI script and top-level README to reference and test the YOLO26 example.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| examples/models/yolo26/requirements.txt | Adds Python deps for the YOLO26 example export/validation. |
| examples/models/yolo26/main.cpp | New YOLO26 C++ demo runner (video in/out + timing + drawing). |
| examples/models/yolo26/inference.h | Updates YOLO post-processing to end-to-end post-NMS [1, N, 6] output parsing. |
| examples/models/yolo26/export_and_validate.py | Updates export/quantization/validation flow and YOLO26 defaults. |
| examples/models/yolo26/README.md | Renames docs and commands from YOLO12 to YOLO26. |
| examples/models/yolo26/CMakeLists.txt | Renames the demo target to Yolo26DetectionDemo. |
| examples/models/yolo12/requirements.txt | Removes YOLO12 example requirements. |
| backends/openvino/quantizer/quantizer.py | Converts tuple-based subgraphs into nncf.Subgraph objects. |
| README.md | Adds YOLO26 to the list of vision/speech examples. |
| .ci/scripts/test_yolo26.sh | Updates CI to install/run YOLO26 export and C++ runner; fixes OpenVINO checkout command. |
Comments suppressed due to low confidence (6)
examples/models/yolo26/inference.h:114
pad_x,pad_y, andscaleare used here for coordinate mapping, butscale_with_padding()returns early without initializing them when the input already matchesimg_dims. That can make these computations undefined in the no-resize case. Ensurescale_with_padding()sets*scale = 1.0fand*pad_x = *pad_y = 0before returning early.
examples/models/yolo26/inference.h:120class_idis read from the model output and then used to indexyolo_config.classeswithout any bounds checking. If the model emits-1/NaN/out-of-range ids (or if the class list doesn't match), this will crash. Add a range check (0 <= class_id < classes.size()) and skip invalid detections (or clamp/log).
examples/models/yolo26/export_and_validate.py:84nncf(andnncf.experimental.torch.fx.quantize_pt2e) is imported unconditionally, even whenquantize=False. That makes the non-quantized OpenVINO export path depend on NNCF being installed. Consider moving these imports inside theif quantize:block so FP32 export works without NNCF.
examples/models/yolo26/export_and_validate.py:274- The error message has a typo ("implmenented") and the suggested flag formatting is inconsistent (
[640,640]vs[640, 640]elsewhere). Fixing the typo and using consistent spacing will make the validation guidance clearer.
examples/models/yolo26/export_and_validate.py:297 validator.deviceis assigned to CPU twice here. This duplication is easy to miss and can confuse future edits (e.g., if one of the assignments is changed). Remove the redundant assignment and keep a single, intentional device configuration.
examples/models/yolo26/inference.h:94num_classesis computed but never used, which will trigger an unused-variable warning under stricter builds. Either remove it or use it for validatingclass_idbefore indexing intoyolo_config.classes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| in the model graph. | ||
| """ | ||
| if subgraphs: | ||
| subgraphs = [nncf.Subgraph(inputs=subgraph[0], outputs=subgraph[1]) for subgraph in subgraphs] |
There was a problem hiding this comment.
The subgraphs conversion is written as a long one-liner and assumes each element is a 2-tuple/list ((inputs, outputs)). For readability (and to avoid formatting/lint issues), consider expanding this into a small loop or multi-line comprehension, and optionally handle the case where callers pass nncf.Subgraph objects directly.
| subgraphs = [nncf.Subgraph(inputs=subgraph[0], outputs=subgraph[1]) for subgraph in subgraphs] | |
| processed_subgraphs: List[nncf.Subgraph] = [] | |
| for subgraph in subgraphs: | |
| if isinstance(subgraph, nncf.Subgraph): | |
| processed_subgraphs.append(subgraph) | |
| else: | |
| inputs, outputs = subgraph | |
| processed_subgraphs.append( | |
| nncf.Subgraph(inputs=inputs, outputs=outputs) | |
| ) | |
| subgraphs = processed_subgraphs |
daniil-lyakhov
force-pushed
the
dl/yolo26_rebase
branch
from
188b696 to
102d9fa
Compare
|
@claude Can you review this PR? |
|
Claude finished @digantdesai's task in 2m 0s —— View job PR Review: [Examples] Yolo26 Sample for OpenVINO/XNNPACK
Overall this is a clean rename+upgrade from YOLO12 to YOLO26 with a nice simplification on the C++ side (the model now outputs post-NMS results, removing the client-side NMS). A few issues to address: Bugs / Correctness1. Uninitialized
int pad_x, pad_y;
float scale;When the early return path is taken, lines 110-113 use these uninitialized values for coordinate mapping, which is undefined behavior. Fix by either initializing them at the declaration site ( 2. No bounds check on result.className = yolo_config.classes[class_id];If the model emits a 3. Unused variable const int num_classes = static_cast<int>(yolo_config.classes.size());This is computed but never used. It will produce a compiler warning with 4. Duplicate validator.device = torch.device("cpu") # line 291
...
validator.device = torch.device("cpu") # line 296The device is set twice. One of these should be removed. CI Script Issues5. The old script didn't require Consider making this check conditional on whether quantization is enabled, or moving it to just before it's used. 6. EXPORTED_MODEL_NAME="${MODEL_NAME}_fp32_${MODE}.pte"This hardcodes Backend Change7. if subgraphs:
subgraphs = [nncf.Subgraph(inputs=subgraph[0], outputs=subgraph[1]) for subgraph in subgraphs]This works but has two minor concerns:
Nits / Minor8. Typo — 9. Stale reference in 10. 11. README typo — SummaryThe main functional concern is the uninitialized variables bug in The overall change is well-structured: the YOLO26 end-to-end output format simplifies the C++ inference code substantially by removing client-side NMS, and the CI script improvement (using a pre-installed OpenVINO instead of building from source) is a good cleanup. |
| @@ -1,2 +0,0 @@ | |||
| ultralytics==8.3.196 | |||
There was a problem hiding this comment.
Do we not want to keep Yolo12?
There was a problem hiding this comment.
I decided to remove the Yolo12 model as the Yolo26 has simpler architecture, should run faster and has same/better accuracy results.
Besides, our colleagues from Ultralytics recommend to avoid the yolo12 altogether
https://docs.ultralytics.com/compare/#community-models-a-note-on-yolo12-and-yolo13
digantdesai
left a comment
digantdesai
left a comment
There was a problem hiding this comment.
LGTM, thanks for the yolo26.
- Please fix issues Claude commented.
- CI is not clean, see if anything relevant needs fixing.
- Left a comment about Yolo12.
|
@digantdesai, Thank you for a quick response! I fixed the claude code comments (double checked with my local cloude code and applied his comments as well) Found actual issue with the precommit - lintrunner, should be fixed |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
daniil-lyakhov
force-pushed
the
dl/yolo26_rebase
branch
from
e31957f to
34e9377
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
.ci/scripts/test_yolo26.sh:85
- The script sources
openvino/setupvars.shand installsbackends/openvino/requirements.txtusing paths relative to the current working directory, and usespipinstead of$PYTHON_EXECUTABLE -m pip. This makes the script fragile if invoked from a different cwd or with a non-default Python; prefer resolving paths relative to the repo root/script directory and use the same Python executable for installs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cv::Mat scale_with_padding( | ||
| const cv::Mat& source, | ||
| int* pad_x, | ||
| int* pad_y, | ||
| float* scale, | ||
| cv::Size img_dims) { | ||
| int col = source.cols; | ||
| int row = source.rows; | ||
| int m_inputWidth = img_dims.width; | ||
| int m_inputHeight = img_dims.height; | ||
| if (col == m_inputWidth && row == m_inputHeight) { | ||
| *pad_x = 0; | ||
| *pad_y = 0; | ||
| *scale = 1.f; | ||
| return source; | ||
| } | ||
|
|
||
| *scale = std::min(m_inputWidth / static_cast<float>(col), m_inputHeight / static_cast<float>(row)); | ||
| int resized_w = static_cast<int>(col * *scale); | ||
| int resized_h = static_cast<int>(row * *scale); | ||
| *pad_x = (m_inputWidth - resized_w) / 2; | ||
| *pad_y = (m_inputHeight - resized_h) / 2; | ||
|
|
||
| cv::Mat resized; | ||
| cv::resize(source, resized, cv::Size(resized_w, resized_h)); | ||
| cv::Mat result = cv::Mat::zeros(m_inputHeight, m_inputWidth, source.type()); | ||
| resized.copyTo(result(cv::Rect(*pad_x, *pad_y, resized_w, resized_h))); | ||
| resized.release(); | ||
| return result; | ||
| } | ||
|
|
||
| std::vector<Detection> infer_yolo_once( | ||
| Module& module, | ||
| cv::Mat input, | ||
| cv::Size img_dims, | ||
| const DetectionConfig& yolo_config) { |
There was a problem hiding this comment.
scale_with_padding and infer_yolo_once are non-inline function definitions in a header. If this header is included by more than one translation unit, it will cause multiple-definition linker errors. Mark these functions inline (or move implementations to a .cpp file).
| #include <iostream> | ||
| #include <vector> | ||
|
|
||
| #include <executorch/extension/module/module.h> | ||
| #include <executorch/extension/tensor/tensor.h> | ||
| #include <executorch/runtime/core/error.h> | ||
| #include <executorch/runtime/core/exec_aten/exec_aten.h> | ||
| #include <executorch/runtime/core/result.h> | ||
| #include <executorch/runtime/platform/runtime.h> | ||
| #include <opencv2/opencv.hpp> |
There was a problem hiding this comment.
This header uses standard library facilities/macros that require their own headers (std::min and PRIx32), but it doesn’t include <algorithm> and <cinttypes>/<inttypes.h>. Add the required standard headers explicitly to avoid relying on transitive includes.
daniil-lyakhov
force-pushed
the
dl/yolo26_rebase
branch
from
844f0e0 to
f82088c
Compare
Summary
Yolo12 is replaced by the Yolo26 model in the examples
Test plan
Local sanity check
test_yolo26.sh is running ok on my local machine, please help me to validate the .ci/scripts/test_yolo26.sh
CC: @mergennachin @lucylq