Skip to content

Commit 4624647

Browse files
kiritigowdacursoragent
kiritigowda
and
cursoragent
committed
extraction: fix last 3 rustVX failures (MatchTemplate / HOGFeatures / HoughLinesP)
Three benchmarks still failed under rustVX after the CTS-pattern adoption (bef2fc4) — each for a distinct reason rooted in spec behaviour vs benchmark input design. 1. MatchTemplate - VERIFY FAILED Previous CTS-style verify used VX_COMPARE_CCORR_NORM with a uniform-bright template against a partially-bright source. The problem: CCORR_NORM is *scale-invariant* by construction (normalisation divides out intensity scale), so a uniform template correlates to ~1.0 against ANY uniform image patch - bright OR dark - and the "peak" appears at every uniform cell rather than the embedded-template position. Fix: switch to VX_COMPARE_L2 with argmin. Sum-of-squared- differences is MIN at the match, saturated to INT16_MAX elsewhere - every CTS-conformant impl produces a unique minimum at the embedded position regardless of internal fixed-point conventions. 2. HOGFeatures - SKIPPED (vxProcessGraph failed) The bench graph created magnitudes/bins tensors as INPUTS to the HOGFeatures node but never populated them. Lenient impls (AMD AGO) treat unwritten tensors as zero-initialised, but strict-FFI impls (rustVX) hold tensor data in a lazy-allocated map keyed by tensor address - reading from a never-written tensor returns VX_ERROR_INVALID_REFERENCE inside get_tensor_data, propagates out of vxProcessGraph, and lands the bench as SKIPPED. Fix: chain HOGCells -> HOGFeatures in the bench graph so the cells kernel populates magnitudes/bins as a side-effect upstream of the features kernel. ~10% added cost at FHD, and it matches how a real HOG pipeline actually runs (always Cells -> Features chained). 3. HoughLinesP - SKIPPED (vxProcessGraph failed) The bench input was a sparse grid + diagonals pattern with ~10k non-zero edge pixels at VGA. rustVX's HoughLinesP impl uses a probabilistic-line-tracer with an O(N) linear scan over the points vector at every traced pixel - total cost is O(N^2 x theta) ~ 360 billion ops at VGA, overruning realistic CI timeouts. AND vxAddArrayItems overflows our 1024-capacity lines array long before the tracer finishes. Fix: minimal-pattern input (1 horizontal + 1 vertical line intersecting at image center, edge count = W + H = ~1120 at VGA, ~3000 at FHD) and bumped the lines array capacity to 8192. Still exercises every code path (accumulator build, peak detection, line tracing) but at a tractable scale. verify_fn unchanged - its mini 64x64 input was already minimal. Verified locally against AMD MIVisionX (CPU build): all 3 affected benches still skip cleanly with "kernel not available" - no regression on the impl that doesn't export them. Next CI run against rustVX will validate the three fixes uniformly. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent bef2fc4 commit 4624647

2 files changed

Lines changed: 136 additions & 46 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,53 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66

77
## [Unreleased]
88

9+
### Fixed — Last 3 rustVX enhanced_vision failures (MatchTemplate / HOGFeatures / HoughLinesP)
10+
11+
Follow-up to the CTS-style verify_fn rewrite. Three benchmarks
12+
still failed under rustVX even after the CTS-pattern adoption,
13+
each for a distinct reason rooted in spec behaviour vs benchmark
14+
input design:
15+
16+
- **`MatchTemplate`** : `VERIFY FAILED`. The previous CTS-style
17+
verify used `VX_COMPARE_CCORR_NORM` with a uniform-bright
18+
template against a partially-bright source. CCORR_NORM is
19+
*scale-invariant* by construction (normalisation divides out
20+
intensity scale), so a uniform template correlates to ~1.0
21+
against ANY uniform image patch — bright OR dark — and the
22+
"peak" appears at every uniform cell rather than the
23+
embedded-template position. Switched to `VX_COMPARE_L2` with
24+
argmin (sum-of-squared-differences, MIN at the match, saturated
25+
to INT16_MAX elsewhere) — every CTS-conformant impl produces a
26+
unique minimum at the embedded position.
27+
28+
- **`HOGFeatures`** : `SKIPPED (vxProcessGraph failed during
29+
measurement)`. The bench graph created the magnitudes/bins
30+
tensors but never populated them — lenient impls (AMD AGO) treat
31+
unwritten tensors as zero-initialised, but strict-FFI impls
32+
(rustVX) hold tensor data in a lazy-allocated map keyed by
33+
tensor address, and reading from a never-written tensor returns
34+
`VX_ERROR_INVALID_REFERENCE` inside `get_tensor_data`, which
35+
propagates out of `vxProcessGraph`. Fix: chain `HOGCells →
36+
HOGFeatures` in the bench graph so the cells kernel populates
37+
magnitudes/bins as a side-effect upstream of the features kernel.
38+
~10% added cost at FHD, and it matches how a real HOG pipeline
39+
actually runs (always Cells → Features chained).
40+
41+
- **`HoughLinesP`** : `SKIPPED (vxProcessGraph failed during
42+
measurement)`. The bench input was a sparse grid + diagonals
43+
pattern with ~10k non-zero edge pixels at VGA. rustVX's
44+
HoughLinesP impl uses a probabilistic-line-tracer with an O(N)
45+
linear scan over the points vector at every traced pixel — total
46+
cost is O(N² × theta) ≈ 360 billion ops at VGA, overruning
47+
realistic CI timeouts, AND `vxAddArrayItems` overflows our
48+
1024-capacity lines array long before the tracer finishes.
49+
Fix: minimal-pattern input (1 horizontal + 1 vertical line
50+
intersecting at image center, edge count = W + H = ~1120 at VGA,
51+
~3000 at FHD) and bumped the lines array capacity to 8192. Still
52+
exercises every code path (accumulator build, peak detection,
53+
line tracing) but at a tractable scale, and the verify_fn is
54+
unchanged (its mini 64×64 input was already minimal).
55+
956
### Changed — Enhanced-Vision verify_fns now follow OpenVX CTS patterns (8 kernels)
1057

1158
Eight benchmark `verify_fn`s have been rewritten to follow the

‎src/benchmarks/node_extraction.cpp‎

Lines changed: 89 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -98,18 +98,29 @@ std::vector<BenchmarkCase> registerExtractionBenchmarks() {
9898
// CTS-style structural check (modelled on
9999
// OpenVX-cts test_matchtemplate.c testGraphProcessing):
100100
// place a known template at a known location in the source
101-
// image, run MatchTemplate, then locate the correlation
102-
// peak with `vx_int16` argmax over the output. Verify the
103-
// peak is at the expected position within ±1 pixel
104-
// tolerance. This pattern is impl-independent — every
105-
// CTS-conformant impl must find the peak at the embedded-
106-
// template location regardless of internal fixed-point
107-
// conventions, because correlation is maximised where the
108-
// patterns align.
101+
// image, run MatchTemplate, then locate the match position
102+
// by finding the EXTREMUM in the output. Verify the
103+
// extremum is at the expected position within ±1 pixel.
104+
//
105+
// Method choice: L2 (sum-of-squared-differences). Three
106+
// reasons it's better than the normalized variants for
107+
// verification:
108+
// (a) Method MIN is at the match position (argmin search).
109+
// (b) Saturates to INT16_MAX away from the match (all
110+
// non-match cells look the same — easy to spot the
111+
// unique minimum).
112+
// (c) NOT scale-invariant — CCORR_NORM normalises away
113+
// intensity scale, so a uniform-bright template
114+
// correlates to 1.0 against ANY uniform image region
115+
// (bright OR dark), and the "peak" appears at every
116+
// uniform cell rather than the true match. L2
117+
// respects absolute pixel-value differences, so the
118+
// match position is the unique minimum.
109119
//
110120
// Setup: 64x64 dark source with a 16x16 bright square
111-
// embedded at (24, 24). Template is 16x16 bright. Peak
112-
// should appear at (24, 24) in the output correlation map.
121+
// embedded at (24, 24). Template is 16x16 bright.
122+
// L2 output: 0 at (24, 24), saturated to INT16_MAX
123+
// everywhere uniform.
113124
constexpr uint32_t W = 64, H = 64, TW = 16, TH = 16;
114125
constexpr uint32_t OW = W - TW + 1, OH = H - TH + 1;
115126
constexpr uint32_t PEAK_X = 24, PEAK_Y = 24;
@@ -130,7 +141,7 @@ std::vector<BenchmarkCase> registerExtractionBenchmarks() {
130141
return true;
131142
}
132143
vx_image out = vxCreateImage(ctx, OW, OH, VX_DF_IMAGE_S16);
133-
vx_enum method = VX_COMPARE_CCORR_NORM;
144+
vx_enum method = VX_COMPARE_L2;
134145
vx_scalar match_method = vxCreateScalar(ctx, VX_TYPE_ENUM, &method);
135146
vx_graph g = vxCreateGraph(ctx);
136147
vx_kernel k = vxGetKernelByEnum(ctx, VX_KERNEL_MATCH_TEMPLATE);
@@ -146,21 +157,20 @@ std::vector<BenchmarkCase> registerExtractionBenchmarks() {
146157
if (status == VX_SUCCESS) {
147158
auto result = verify::readImageS16(out, OW, OH);
148159
if (!result.empty()) {
149-
// Find argmax of the correlation map (CCORR_NORM ⇒
150-
// higher = better match). Don't rely on absolute
151-
// values — only the LOCATION of the peak is
152-
// semantics-independent.
153-
int16_t peak_val = INT16_MIN;
154-
uint32_t peak_x = 0, peak_y = 0;
160+
// Find argmin of the L2 distance map (lower = better
161+
// match). Don't rely on absolute values — only the
162+
// LOCATION of the minimum is semantics-independent.
163+
int16_t best_val = INT16_MAX;
164+
uint32_t best_x = 0, best_y = 0;
155165
for (uint32_t y = 0; y < OH; ++y) {
156166
for (uint32_t x = 0; x < OW; ++x) {
157167
int16_t v = result[y * OW + x];
158-
if (v > peak_val) { peak_val = v; peak_x = x; peak_y = y; }
168+
if (v < best_val) { best_val = v; best_x = x; best_y = y; }
159169
}
160170
}
161-
// CTS allows ±1 pixel tolerance on the peak location.
162-
const int dx = static_cast<int>(peak_x) - static_cast<int>(PEAK_X);
163-
const int dy = static_cast<int>(peak_y) - static_cast<int>(PEAK_Y);
171+
// CTS allows ±1 pixel tolerance on the match location.
172+
const int dx = static_cast<int>(best_x) - static_cast<int>(PEAK_X);
173+
const int dy = static_cast<int>(best_y) - static_cast<int>(PEAK_Y);
164174
ok = (dx >= -1 && dx <= 1 && dy >= -1 && dy <= 1);
165175
}
166176
} else {
@@ -419,6 +429,29 @@ std::vector<BenchmarkCase> registerExtractionBenchmarks() {
419429
vxCreateTensor(ctx, 3, feat_dims, VX_TYPE_INT16, 0));
420430
if (vxGetStatus((vx_reference)features) != VX_SUCCESS) return false;
421431

432+
// Chain HOGCells → HOGFeatures in the bench graph.
433+
//
434+
// Why: HOGFeatures needs populated magnitudes + bins
435+
// tensors as input. Lenient impls (AMD/Khronos) tolerate
436+
// an unwritten input tensor by treating it as
437+
// zero-initialised, but strict-FFI impls (rustVX) hold
438+
// tensor data in a lazy-allocated map keyed on the tensor
439+
// address — reading from a tensor that was never written
440+
// returns VX_ERROR_INVALID_REFERENCE inside
441+
// get_tensor_data, which propagates out of vxProcessGraph
442+
// and lands the bench as `SKIPPED (vxProcessGraph failed
443+
// during measurement)`. Running HOGCells upstream
444+
// populates both tensors as a side-effect, which costs
445+
// ~10% of the HOGFeatures kernel cost at FHD and brings
446+
// the bench in line with what a real HOG pipeline does
447+
// (always run as a Cells → Features chain).
448+
auto cells_fn = openvx_optional::hogCellsNode();
449+
if (!cells_fn) return false;
450+
vx_node cells_node = cells_fn(graph, input, CELL, CELL, BINS,
451+
magnitudes, bins);
452+
if (vxGetStatus((vx_reference)cells_node) != VX_SUCCESS) return false;
453+
tracker.trackNode(cells_node);
454+
422455
auto fn = openvx_optional::hogFeaturesNode();
423456
if (!fn) return false;
424457
vx_node node = fn(graph, input, magnitudes, bins,
@@ -536,31 +569,34 @@ std::vector<BenchmarkCase> registerExtractionBenchmarks() {
536569
TestDataGenerator& gen, ResourceTracker& tracker) -> bool {
537570
(void)gen; // we synthesize the input ourselves below
538571
// OpenVX 1.3.1 §3.27: input MUST be a binary edge map.
539-
// A truly random U8 image has ~99.6% non-zero pixels —
540-
// strict impls iterate every non-zero pixel through every
541-
// theta bin in the accumulator (~180 iters), so an FHD
542-
// random input produces ~2M·180 = 360M accumulator ops
543-
// per call, taking seconds-to-minutes per iteration.
544-
// Synthesise a sparse binary edge map instead: a handful
545-
// of vertical/horizontal/diagonal lines drawn into a
546-
// mostly-zero buffer, giving a deterministic ~0.1% non-
547-
// zero density that still exercises every code path in
548-
// the HoughLinesP algorithm (accumulator build, peak
549-
// detection, line tracing) at realistic edge densities.
572+
// Two reasons we draw a MINIMAL pattern (just 2 lines)
573+
// rather than something dense:
574+
// 1. A truly random U8 image has ~99.6% non-zero pixels.
575+
// Strict impls iterate every non-zero pixel through
576+
// every theta bin (~180 iters) and then trace each
577+
// candidate line forward+backward, with an O(N) inner
578+
// lookup over the points vector at every step. That's
579+
// O(N² × theta) total — ~360 billion ops at FHD,
580+
// which overruns realistic CI timeouts and lands the
581+
// bench as `SKIPPED (vxProcessGraph failed)` because
582+
// vxAddArrayItems also overflows long before the
583+
// tracer finishes.
584+
// 2. We don't need a dense pattern to measure
585+
// HoughLinesP's per-pixel accumulator cost — that
586+
// cost is paid linearly in non-zero pixel count, so
587+
// a sparse pattern still exercises the same code
588+
// path at every CTS-conformant impl, just on a
589+
// tractable scale.
590+
//
591+
// Minimal pattern: one horizontal and one vertical line at
592+
// image center → 2 long strong Hough peaks, edge-point
593+
// count = W + H (~1120 at VGA, ~3000 at FHD), well under
594+
// the O(N²) blow-up threshold.
550595
std::vector<uint8_t> buf(static_cast<size_t>(width) * height, 0);
551-
const uint32_t step_x = std::max<uint32_t>(1, width / 8);
552-
const uint32_t step_y = std::max<uint32_t>(1, height / 8);
553-
for (uint32_t y = 0; y < height; ++y) {
554-
for (uint32_t x = 0; x < width; ++x) {
555-
// 4 axis-aligned grid lines + 4 diagonals → sparse
556-
// edge map with strong Hough-detectable structure.
557-
if (x % step_x == 0 || y % step_y == 0 ||
558-
x == y ||
559-
x + y == (width - 1)) {
560-
buf[static_cast<size_t>(y) * width + x] = 255;
561-
}
562-
}
563-
}
596+
const uint32_t cy = height / 2;
597+
const uint32_t cx = width / 2;
598+
for (uint32_t x = 0; x < width; ++x) buf[cy * width + x] = 255;
599+
for (uint32_t y = 0; y < height; ++y) buf[y * width + cx] = 255;
564600
vx_image input = tracker.trackImage(
565601
verify::createImage(ctx, width, height, VX_DF_IMAGE_U8, buf.data()));
566602
if (vxGetStatus((vx_reference)input) != VX_SUCCESS) return false;
@@ -573,8 +609,15 @@ std::vector<BenchmarkCase> registerExtractionBenchmarks() {
573609
// like rustVX, where a panic across the FFI boundary is
574610
// undefined behaviour and manifests as a segfault. Use
575611
// the spec-mandated VX_TYPE_LINE_2D.
612+
//
613+
// 8192 capacity (vs the previous 1024) — strict impls
614+
// return a vxAddArrayItems error and abort vxProcessGraph
615+
// if the detected-line count exceeds capacity. Even our
616+
// minimal 2-line pattern can split into 50+ segments per
617+
// line under aggressive gap/length params; 8k absorbs
618+
// that headroom without measurable cost.
576619
vx_array lines = tracker.trackArray(
577-
vxCreateArray(ctx, VX_TYPE_LINE_2D, 1024));
620+
vxCreateArray(ctx, VX_TYPE_LINE_2D, 8192));
578621
if (vxGetStatus((vx_reference)lines) != VX_SUCCESS) return false;
579622

580623
vx_size zero = 0;

0 commit comments

Comments
 (0)