Fix vxMinMaxLocNode minCount/maxCount scalar type to VX_TYPE_SIZE (#29)#39
Merged
Merged
Conversation
Per the OpenVX 1.3 specification, the minCount and maxCount parameters of vxMinMaxLocNode must be VX_TYPE_SIZE scalars. The conformance test was creating them as VX_TYPE_UINT32, which does not match the specified API contract and can cause false failures against spec-correct implementations (and can mask non-conformant ones), especially on LP64 platforms where vx_size is 8 bytes. Change the reference helper and test variables from uint32_t to vx_size and create the count scalars as VX_TYPE_SIZE. Diagnostic printf counts are cast to int to avoid -Wformat warnings. Co-authored-by: Cursor <cursoragent@cursor.com>
Port the internal GitLab pipeline (.gitlab-ci.yml) to GitHub Actions so PRs run the CTS against a freshly built KhronosGroup/OpenVX-sample-impl. A sanity build gates a test matrix covering Vision, Vision+Enhanced, Neural Networks, Combined, User Data Object, Pipelining, plus dedicated NNEF Import and warn-only Streaming jobs. Bakes in known build gotchas: install libs under bin/ (CMAKE_INSTALL_BINDIR/ LIBDIR), force-include <limits> for the NNEF submodule, link libnnef-lib.a + stdc++ for NNEF, exclude AlexNet (weights not shipped), and treat streaming (stubbed APIs) as a warning. Co-authored-by: Cursor <cursoragent@cursor.com>
The AlexNet network test data is present in the repo (weights under test_conformance/Networks/Binaries/Alexnet and input images under test_data/images), and the test resolves them via VX_TEST_DATA_PATH/.. which maps correctly in CI. Remove the AlexNet exclusion filter from the Neural Networks and Combined jobs so they run the complete suite. Co-authored-by: Cursor <cursoragent@cursor.com>
kiritigowda
added
1.3.2
bug
Since openvx_1.3.2, test_data/ binary images are stored via Git LFS. A plain clone without Git LFS silently downloads pointer files, and the CTS then fails at runtime with "Can't open image file" errors. Add a Prerequisites entry and a Cloning section explaining how to install Git LFS and recover an existing clone with `git lfs pull`. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Aligns the CTS MinMaxLoc conformance test with the OpenVX 1.3 requirement that minCount/maxCount be VX_TYPE_SIZE scalars, and adds supporting documentation + CI to run conformance in GitHub Actions.
Changes:
- Update MinMaxLoc reference/test code to use
vx_sizecounts and createVX_TYPE_SIZEscalars. - Document Git LFS requirement for fetching binary test images in
test_data/. - Add a GitHub Actions workflow to build the sample implementation and run conformance matrices.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test_conformance/test_minmaxloc.c | Switches MinMaxLoc count variables/scalars to vx_size/VX_TYPE_SIZE and adjusts diagnostics. |
| README.md | Adds Git LFS setup instructions for test data. |
| .github/workflows/conformance.yml | Introduces CI jobs to build sample impl + run CTS in multiple modes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- conformance.yml: set `lfs: true` on every actions/checkout step so the Git LFS-managed test_data/ images are actually fetched (otherwise the CTS fails at runtime opening images like lena.bmp). GitHub-hosted runners ship with git-lfs preinstalled. - test_minmaxloc.c: print mincount/maxcount with %zu and cast to size_t instead of truncating vx_size to int on LP64 platforms. Co-authored-by: Cursor <cursoragent@cursor.com>
The MatchTemplate conformance test locates the correlation peak via vxMinMaxLocNode/vxuMinMaxLoc, creating the minCount/maxCount scalars as VX_TYPE_UINT32. Per the OpenVX 1.3 spec these must be VX_TYPE_SIZE; against a spec-conformant implementation that validates the scalar type, graph verification rejects the VX_TYPE_UINT32 scalars with VX_ERROR_INVALID_TYPE, failing all MatchTemplate.GraphProcessing/ImmediateProcessing cases. Create the count scalars as VX_TYPE_SIZE and widen the mincount/maxcount locals to vx_size, matching the MinMaxLoc test change. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Summary
The OpenVX 1.3 specification requires the minCount and maxCount parameters of �xMinMaxLocNode to be
VX_TYPE_SIZEscalars:The conformance test in
test_conformance/test_minmaxloc.cwas instead creating these scalars asVX_TYPE_UINT32. This does not match the specified API contract and can:VX_TYPE_SIZEat graph verification, andVX_TYPE_UINT32.This matters in practice on LP64 platforms (Linux/macOS/ARM64) where
vx_sizeis 8 bytes vs 4 foruint32_t.Changes
reference_minmaxloc()count parameters:uint32_t*->vx_size*testOnRandom:uint32_t->vx_sizeVX_TYPE_SIZEinstead ofVX_TYPE_UINT32printfcount values cast tointto avoid-Wformatwarnings on 64-bit buildsThis is the
#29fix originally proposed in #36, applied in isolation.Test plan
*MinMaxLoc*conformance tests against a 1.3.2 implementation on x86_64 and ARM64Made with Cursor