executorch export docs: input contiguity - #1237
Merged
Borda merged 14 commits intoJul 27, 2026
Merged
Conversation
roboflow#1233) infer_transforms returns a channels_last (non-contiguous) tensor. The ExecuTorch runtime ignores input strides and reads the buffer as contiguous NCHW, so the image reaching the model is scrambled and detections collapse. The existing torch.randn parity check cannot catch this: a freshly allocated random tensor is already contiguous. Constant and all-zero inputs are likewise layout-invariant, so only a structured image passed through the real preprocessing pipeline exercises the fault. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The ExecuTorch runtime reads its input buffer as contiguous NCHW and ignores tensor strides. torchvision's ToImage permutes a decoded HWC buffer to CHW without copying, so infer_transforms emitted a channels_last view; the runtime misread it as a scrambled image and returned wrong predictions with no error. On a pretrained model every detection collapsed below threshold. Force the copy in infer_transforms so all export inference paths pick it up, and document the requirement in the ExecuTorch inference example, whose np.transpose produced the same strided view. Fixes roboflow#1233 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces the drawn synthetic image with ImageAssets.SOCCER, a real photograph, so the end-to-end check runs on the kind of input users actually feed the runtime. Skips when media.roboflow.com is unreachable, matching the offline guard used by the COCO benchmark fixtures. The contiguity unit test keeps a synthetic image and stays offline: ToImage permutes a decoded HWC buffer to CHW as a view regardless of pixel content, so that assertion does not need a download in the fast CPU suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
supervision's ImageAssets only ships people-walking.jpg and soccer.jpg -- there is no dog asset (the image-examples path 404s for it). Fetch the dog photograph the project README already uses in its inference examples instead, via requests with the same offline socket guard and timeout convention as tests/inference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This reverts commit cde863e. supervision's ImageAssets has no dog image, so switching to one meant dropping the supervision asset API for a direct download. Keep ImageAssets.SOCCER, the only real photograph that API offers, rather than fetching an image by URL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
anatoly-ryabchenko
requested review from
Borda,
SkalskiP,
isaacrob and
probicheaux
as code owners
July 27, 2026 09:21
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (84%) is below the target coverage (95%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #1237 +/- ##
=======================================
- Coverage 84% 84% -0%
=======================================
Files 103 103
Lines 13250 13252 +2
=======================================
+ Hits 11113 11114 +1
- Misses 2137 2138 +1 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses ExecuTorch (XNNPACK) export inference correctness issues caused by non-contiguous input tensors by (1) enforcing contiguity in the shared export benchmarking preprocessing pipeline, (2) adding regression tests that cover real-image parity and transform contiguity, and (3) documenting the contiguity requirement for users running .pte programs.
Changes:
- Ensure
infer_transformsreturns a contiguous CHW tensor by appending a contiguity materialization step. - Add ExecuTorch end-to-end parity regression coverage using a real photo input plus a unit test asserting
infer_transformsoutput contiguity. - Update export docs with an explicit warning and example fix (
np.ascontiguousarray/Tensor.contiguous()).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/export/test_executorch_export.py |
Adds regression tests for real-photo parity and infer_transforms contiguity to prevent silent ExecuTorch stride-misread failures. |
src/rfdetr/export/benchmark.py |
Forces contiguous tensors at the end of infer_transforms so export/benchmark inference paths produce runtime-safe inputs. |
docs/learn/export.md |
Documents the ExecuTorch runtime’s contiguity requirement and demonstrates how to ensure a contiguous NCHW input. |
Member
|
@anatoly-ryabchenko, so can we not add in the cookbook also inference on the CPU machine to show output detection? |
The ExecuTorch cookbook stopped at export: section 5 was a markdown reference block, and a top-of-notebook warning declared on-device inference blocked by an ExecuTorch lowering bug said to corrupt the DINOv2 backbone output. That diagnosis was wrong. The reference code fed the runtime the tensor straight out of infer_transforms, which was a channels_last view; the ExecuTorch runtime reads its input buffer as contiguous NCHW and ignores strides, so the model saw a scrambled image (issue roboflow#1233). Making the tensor contiguous fixes it -- the .pte matches eager PyTorch and returns correct detections. Turn the reference block into live code: download a sample image, run the .pte on the CPU, and annotate the detections with supervision, mirroring the TensorRT cookbook. Replace the incorrect warning with the contiguity requirement, and drop the "for working inference today, use TensorRT" note. Verified by executing every code cell end to end: dog 0.96, backpack 0.63, person 0.58, car 0.52. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…uity' into fix/1233-executorch_input_contiguity
Contributor
Author
Done |
supervision's annotators accept a numpy array or a PIL image and return the same type, but a numpy scene is assumed to be OpenCV-style BGR. Passing np.array(image) handed them an RGB buffer, so the drawn boxes came out with red and blue swapped (palette violet 163,81,251 rendered as 251,81,163) and sv.plot_image, which runs COLOR_BGR2RGB on its input, then swapped the photo itself. Annotate the PIL image directly so supervision converts in both directions. Verified by sampling the drawn border against ColorPalette.DEFAULT: the numpy path yields (251, 81, 163), the PIL path (163, 81, 251). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running the ExecuTorch or TensorRT cookbook from docs/cookbooks/ writes its EXPORT_DIR there, leaving a 100+ MB .pte or .trt next to the notebook. Neither directory was ignored, so a stray `git add` picks up model weights. Mirrors the existing docs/cookbooks/output/ entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Borda
previously approved these changes
Jul 27, 2026
- Clarify install/setup steps for CoreML and QNN backends. - Improve section formatting with proper headings and syntax. - Consolidate export commands under clearly labeled examples.
Resolve /review findings on PR roboflow#1237 (report 2026-07-27T11-48-47Z): - notebook H1 -> "ExecuTorch Export & Inference", matching the cards.yaml / NOTES.md rename (finding roboflow#2) - notebook warning + section 6: state the contiguity fix lives in infer_transforms (_ensure_contiguous) and label the notebook's own .contiguous() call defensive / no-op, not load-bearing (finding roboflow#1) - notebook + export.md: replace "confident-looking but wrong predictions ... below threshold" with "plausible-shaped output, but every detection's score collapses below threshold" (finding roboflow#6) export.md's manual-numpy example keeps its np.ascontiguousarray note unchanged — that path genuinely permutes without copying. --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Resolve /review finding roboflow#7 on PR roboflow#1237: the network-reachability probe was copied verbatim into three test modules. - add tests/_online.py exposing is_online() - drop the local _is_online definitions in tests/benchmarks/conftest.py, tests/inference/test_predict.py and tests/export/test_executorch_export.py (and their now-unused `import socket`), importing the shared helper - add "." to pytest pythonpath so `from tests._online import is_online` resolves from every test directory, including tests/benchmarks (no __init__.py) Verified: tests/export + tests/inference offline suite 64 passed / 3 skipped; tests/benchmarks collects 41 tests. --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Related Issue(s): closes #1233
Type of Change
Testing
Test details:
Added 2 tests:
Checklist