feat(kpm): port HoughSimilarityVoting::autoAdjustXYNumBins (#150) by kalwalt · Pull Request #151 · webarkit/WebARKitLib-rs

kalwalt · 2026-05-20T16:47:16Z

Summary

Implements #150 — ports C++ HoughSimilarityVoting::autoAdjustXYNumBins (hough_similarity_voting.cpp:204-236) so the Rust HoughSimilarityVoting auto-sizes its x/y bin grid from the median projected dimension of input matches per voting pass. Adds the missing fast_median_f32 + partial_sort_f32 primitives that the auto-adjust formula needs. make_hough_voter switches to BinParams::new_auto_xy, removing the M9-1 vestigial 12×12 bin constants.

Metric	M9-1 baseline	After #146	After #150 (this PR)
`HoughSimilarityVoting::autoAdjustXYNumBins` ported	❌	❌	✅ byte-equivalent to C++
`fast_median_f32` / `partial_sort_f32` ported	❌	❌	✅ byte-equivalent to C++
`BinParams` API: auto-adjust factory	❌	❌	✅ `new_auto_xy`
`HOUGH_NUM_X_BINS = 12` (Rust-only workaround)	present	present	removed
Tests (lib, no features)	~395	395	407
Tests (lib, --features dual-mode)	~417	420	431
End-to-end parity gate `diff`	15	15	15 (unchanged — see R2)

Detailed Description

Pre-brainstorm finding

C++ HoughSimilarityVoting::autoAdjustXYNumBins is private (hough_similarity_voting.h:302) and mNumXBins / mNumYBins have no public getters. The dual-mode FFI shim sidesteps the access issue by reimplementing the formula using public primitives vision::SafeDivision + vision::FastMedian, testing the same arithmetic without needing private state access. No friend declaration; no third-party patches.

Two-layer dual-mode test diagnostics (both passing byte-equivalently)

Test	Trials	Status
`math::dual_mode_tests::dual_mode_partial_sort_f32_matches_cpp`	50 (seeded random + injected duplicates)	✅ pass
`hough::dual_mode_tests::auto_adjust_xy_num_bins_matches_cpp`	40 (seeded random with varied dims)	✅ pass

The lower-level partial_sort_f32 test localizes any future algorithm-level regression; the auto-adjust test verifies the surrounding-math composition.

What changed, by file

math.rs (+196 LOC) — pub fn fast_median_f32(values: &mut [f32]) -> f32 and private partial_sort_f32 helper. Direct port of C++ FastMedian<T> (single-value overload). Preserves the C++ "biased estimator" quirk: returns the (n/2 - 1)-th smallest element (0-indexed), NOT the true mathematical median. For [1,2,3,4,5] returns 2.0, not 3.0. Documented thoroughly. +6 unit tests.
hough.rs (+437 LOC net) — BinParams API expansion:
- num_x_bins and num_y_bins become private (grep-verified safe: no external readers/writers).
- New pub fn num_x_bins() -> i32 / pub fn num_y_bins() -> i32 getters.
- New pub fn new_auto_xy(...) factory (initializes both bins to the clamp floor 5, sets auto_adjust_xy: bool = true).
- New pub(crate) fn set_xy_bins(x, y) atomic mutator that recomputes a / b strides.
- New private auto_adjust_xy: bool field.
- New private HoughSimilarityVoting::recompute_xy_bins_from_matches mirrors C++ autoAdjustXYNumBins via fast_median_f32 + safe_division_f32.
- find_hough_similarity invokes it when the flag is set, before the vote loop.
- +3 unit tests, +1 dual-mode test.
visual_database.rs (+34 LOC net) — HOUGH_NUM_X_BINS / HOUGH_NUM_Y_BINS constants removed (M9-1 vestigial; C++ has no equivalent — it passes 0 to trigger auto-adjust). make_hough_voter switches to BinParams::new_auto_xy. The parity test test_visual_database_matches_cpp_pipeline gets an updated docstring naming the now-confirmed root cause of the residual gap.
kpm_c_api.h + kpm_c_api.cpp (+96 LOC) — two new diagnostic FFI shims. webarkit_cpp_partial_sort_f32 wraps vision::PartialSort<float> directly. webarkit_cpp_auto_adjust_xy_num_bins reimplements the C++ formula using public primitives because the original C++ method is private.
docs/design/m9-hough-auto-adjust-xy-bins.md (NEW, 370 lines) — full brainstorming outcome: 10 decisions, 4 assumptions, 3 risks with post-implementation status, complete algorithm reference, post-PR parity diagnosis.

Review Checklist

Algorithm parity

fast_median_f32 returns the (n/2 - 1)-th smallest element (NOT the true median). Test values intentionally encode this C++ quirk.
partial_sort_f32 uses Wirth's k-smallest with 1-indexed k (matches C++ PartialSort<T>).
recompute_xy_bins_from_matches is invoked once per find_hough_similarity call (matches C++ batched vote(ins, ref, size) semantics), not per individual vote(x, y, angle, scale).
BinParams::new_auto_xy initializes num_x_bins = num_y_bins = 5 (the clamp floor), keeping the struct in a valid state pre-recompute.
Auto-adjust no-op'd when matches.is_empty() or bin_size <= 0.0 (defensive).

API hygiene

num_x_bins / num_y_bins private with getters — locks down the stride invariant.
set_xy_bins is pub(crate) — only callable from the recompute method.
All internal hough.rs callsites that read params.num_x_bins / num_y_bins still compile (verified — same-module access is unrestricted).

Tests

7 new tests in math.rs (mod tests).
1 new dual-mode test in math.rs (mod dual_mode_tests).
3 new unit tests in hough.rs (mod tests).
1 new dual-mode test in hough.rs (mod dual_mode_tests).
All existing M7 hough tests still pass with the BinParams visibility change.

Documentation

Design doc docs/design/m9-hough-auto-adjust-xy-bins.md exists with full decision log + post-implementation risk status.
Every new public symbol has a C equivalent: doc line.

Risk Assessment (post-implementation)

#	Risk	Status	What actually happened
R1	`partial_sort_f32` tie-break may diverge from C++	✅ Did not materialize	Lomuto partition port worked first try. 50/50 dual-mode trials byte-equivalent.
R2	Parity gate may still drift > 5 inliers	⚠️ Materialized differently than predicted	`diff=15` unchanged from M9-1 / M9 #146 baselines. Auto-adjust port is byte-equivalent to C++ (40/40 trials), but runs on different inputs because BHC produces different matches (M9 #146 R1 — cross-language `unordered_map` tree-topology nondeterminism).
R3	C++ `autoAdjustXYNumBins` may be private	✅ Did not materialize (workaround)	It is private; shim reimplements the formula using public primitives. Clean sidestep.

Test Coverage

Test module	Pre-#150	Post-#150
`math::tests` (kpm::freak)	48	55 (+7: 6 fast_median variants + 1 partial_sort_f32 pivot)
`math::dual_mode_tests`	5	6 (+1: `dual_mode_partial_sort_f32_matches_cpp`)
`hough::tests`	14	17 (+3: auto-xy init, atomic strides, recompute known/clamp/empty)
`hough::dual_mode_tests`	(new module)	1 (`auto_adjust_xy_num_bins_matches_cpp`)
Total lib tests, default	395	407
Total lib tests, dual-mode	420	431
Ignored	2	4 (+2 diagnostic ignores already documented)

Visual Aid: pipeline before/after

BEFORE (M9-1 + #146)
                                                  fixed 12×12 grid
                                                  (Rust-only workaround)
                                                         ↓
make_hough_voter ─►  BinParams::new(12, 12, 12, 10, ...)  ─►  HoughSimilarityVoting
                                                                       │
                            find_hough_similarity (per query, ref) ────┘


AFTER (#150)
                                                  no x/y bin args; auto_adjust_xy=true
                                                                         ↓
make_hough_voter ─►  BinParams::new_auto_xy(12, 10, ...)  ─►  HoughSimilarityVoting
                                                                       │
              find_hough_similarity (per query, ref):
                  ├── recompute_xy_bins_from_matches  ←─── M9 #150 NEW
                  │     • per-match: safe_division(q.scale, r.scale) × max_dim
                  │     • fast_median over projected_dim
                  │     • bin_size = 0.25 × median
                  │     • num_x_bins, num_y_bins = max(5, ceil(range / bin_size))
                  ├── voting.reset()
                  └── per-match vote loop (unchanged)

Size & Splitting Notes

Total: 5 modified files + 1 new design doc, ~1000 lines net change. Tight scope around #150's three pieces (median primitives, BinParams API, auto-adjust algorithm + invocation). Splitting further wouldn't help:

Splitting fast_median_f32 from auto-adjust would land a primitive with no callers.
Splitting BinParams API expansion from the auto-adjust invocation would land an unused factory.
The FFI shims exist solely for the diagnostic tests, which exist solely to validate the port — natural unit.

Review Automation Findings

Check	Outcome
`cargo fmt --all -- --check`	✅ clean
`cargo build --all-features`	✅ clean
`cargo clippy --all-targets --all-features -- --deny warnings`	✅ 0 new warnings in modified files (pre-existing project warnings unchanged)
`cargo test --lib`	✅ 407 passed, 2 ignored
`cargo test --features dual-mode --lib`	✅ 431 passed, 4 ignored
`cargo test --features dual-mode -- dual_mode_partial_sort_f32_matches_cpp`	✅ pass (50/50 trials)
`cargo test --features dual-mode -- auto_adjust_xy_num_bins_matches_cpp`	✅ pass (40/40 trials)
`cargo test --features dual-mode -- test_visual_database_matches_cpp_pipeline`	⚠️ `#[ignore]`d at `diff=15` (root cause: BHC tree-topology nondeterminism, M9 #146 R1)

What this PR ruled out — diagnostic value

By isolating auto-adjust and proving it byte-equivalent to C++ at the algorithm level (40/40 trials with seeded random inputs), this PR rules out auto-adjust as the cause of the residual diff=15 gap. The remaining divergence is now narrowed to the BHC tree-topology nondeterminism that has persisted since M9-1.

The path forward to closing the parity gate requires one of:

(a) Patch the C++ source to use std::map instead of std::unordered_map for child iteration during BHC build (third_party submodule change).
(b) Vendor a fork of WebARKitLib with that change applied (maintenance burden).
(c) Redefine the parity metric to something less sensitive to tree-topology variance — e.g. pose-estimation accuracy (the actual downstream consumer of the matches) or inlier ratio (matches per total candidates), rather than absolute inlier count.

These will be addressed in a separate architectural follow-up issue.

Refs: #139 (M9 parent), #140 / #145 (M9-1), #146 / #149 (M9 BHC architecture).

Closes #150 — auto-adjust algorithmic port is complete and verified byte-equivalent to C++. The end-to-end dual-mode parity gate test_visual_database_matches_cpp_pipeline remains #[ignore]d for a structural reason that's out of scope for #150.

🤖 Generated with Claude Code

Implements issue #150 — port the C++ auto-adjusting x/y bin grid for Hough similarity voting (visual_database.h:312 + hough_similarity_voting.cpp:204-236). Adds the missing `fast_median_f32` + `partial_sort_f32` primitives and wires auto-adjust into find_hough_similarity so make_hough_voter no longer needs the hand-tuned 12x12 bin grid that M9-1 used as a placeholder. Pre-brainstorm finding: the C++ HoughSimilarityVoting::autoAdjustXYNumBins method is `private` (verified in hough_similarity_voting.h:302). No public getter exposes the resulting `mNumXBins` / `mNumYBins` either. The dual-mode FFI shim sidesteps the access issue by reimplementing the formula using public primitives `vision::SafeDivision` + `vision::FastMedian`, testing the same arithmetic without needing private state access. Changes ------- * math.rs (+196): new pub fn `fast_median_f32(values: &mut [f32]) -> f32` + private `partial_sort_f32` helper. Direct port of C++ `FastMedian<T>` (single-value overload). Preserves the C++ "biased estimator" quirk: returns the (n/2 - 1)-th smallest element (0-indexed), NOT the true median. For [1,2,3,4,5] returns 2.0, not 3.0. Documented thoroughly. +6 unit tests covering odd/even/single/n=100/two-element/pivot-position. * hough.rs (+437 net): BinParams API expansion — `num_x_bins` and `num_y_bins` become private (grep-verified no external readers/writers). New pub `num_x_bins()` / `num_y_bins()` getters. New pub `new_auto_xy(...)` factory (initializes both to clamp floor 5, sets `auto_adjust_xy: bool = true`). New pub(crate) `set_xy_bins(x, y)` atomic mutator that recomputes `a` / `b` strides. New private `auto_adjust_xy` field on BinParams. New private `HoughSimilarityVoting::recompute_xy_bins_from_matches` mirrors C++ autoAdjustXYNumBins via fast_median_f32 + safe_division_f32. `find_hough_similarity` invokes it when the flag is set, before the vote loop. +3 unit tests (initial state, atomic stride update, known-input/clamp/empty cases) + 1 dual-mode test. * visual_database.rs (+34 net): removed `HOUGH_NUM_X_BINS` / `HOUGH_NUM_Y_BINS` constants (M9-1 vestigial; C++ has no equivalent — it passes 0 to trigger auto-adjust). `make_hough_voter` switches to `BinParams::new_auto_xy`. The parity test `test_visual_database_matches_cpp_pipeline` updated with the new diagnosis (see R2 below). * kpm_c_api.h + kpm_c_api.cpp (+96): two new FFI shims — `webarkit_cpp_partial_sort_f32` (D10 lower-level diagnostic) and `webarkit_cpp_auto_adjust_xy_num_bins` (D4 auto-adjust isolation). The auto-adjust shim reimplements the formula directly using public primitives because the C++ method is private. * docs/design/m9-hough-auto-adjust-xy-bins.md (NEW, 370 lines): full brainstorming outcome — 10 decisions, 4 assumptions, 3 risks with post-implementation status, complete algorithm reference, post-PR parity diagnosis. Two layered dual-mode tests — both passing byte-equivalently ----------------------------------------------------------- * `math::dual_mode_tests::dual_mode_partial_sort_f32_matches_cpp` — 50 seeded random trials, including injected duplicates to stress the tie-break. Confirms `partial_sort_f32` produces byte-identical k-th order statistic to `vision::PartialSort<float>`. * `hough::dual_mode_tests::auto_adjust_xy_num_bins_matches_cpp` — 40 seeded random trials with varied (size, ref dims, x/y ranges). Confirms `recompute_xy_bins_from_matches` produces byte-identical `(num_x_bins, num_y_bins)` to C++ `autoAdjustXYNumBins`. Risks materialized / did not materialize ---------------------------------------- * R1 (DID NOT materialize). `partial_sort_f32` is byte-equivalent to C++ first try; the Lomuto partition port worked correctly. The R1 two-layer detection added value as proof rather than as a fallback trigger. * R2 (MATERIALIZED differently than predicted). The `test_visual_database_matches_cpp_pipeline` end-to-end parity gate STILL shows `diff=15` inliers — identical to the M9-1 and M9 #146 baselines. The auto-adjust port is correct (proven byte-equivalent at the algorithm level), but the residual gap is upstream: BHC produces different match sets in Rust vs C++ (the unresolved cross-language tree-topology nondeterminism from M9 #146 R1, caused by unordered_map iteration in both languages). Auto-adjust runs on different inputs and consequently produces different bin counts even though the formula is identical. Resolution: re-#[ignore] the parity test with a thorough docstring naming the now-confirmed root cause (BHC tree-topology nondeterminism). The originally-planned `skip-parity-gate` cargo feature was added then removed — the unconditional #[ignore] made the cfg_attr soft-skip redundant. * R3 (DID NOT materialize). C++ `autoAdjustXYNumBins` is indeed private, but the shim sidesteps cleanly by reimplementing the formula with public primitives. No `friend` declaration; no third-party patches. Verification (CLAUDE.md §5) --------------------------- cargo fmt --all -- --check clean cargo build --all-features --offline clean cargo clippy --all-targets --all-features 0 new warnings in modified files cargo test --lib --offline 407 passed, 2 ignored cargo test --features dual-mode --lib --offline 431 passed, 4 ignored What we ruled out — diagnostic value ------------------------------------ By isolating auto-adjust and proving it byte-equivalent to C++ at the algorithm level, this PR rules it out as the cause of the residual gap. The remaining divergence is now narrowed to the BHC tree-topology nondeterminism that has persisted since M9-1. The path forward to closing the parity gate requires either (a) patching the C++ source to use std::map instead of std::unordered_map for child iteration, (b) vendoring a fork with that change, or (c) redefining the parity metric to something less sensitive to tree-topology variance (e.g. pose accuracy or inlier ratio). To be addressed in a separate architectural issue. Refs: #139 (M9 parent), #140 (M9-1 baseline), #146 / #149 (M9 BHC architecture, R1 origin). Closes #150 — auto-adjust algorithmic port is complete and verified byte-equivalent to C++. The dual-mode parity gate test_visual_database_matches_cpp_pipeline remains #[ignore]d for a structural reason that's out of scope for #150. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

Implements issue #152 — close the M9 dual-mode parity gate test_visual_database_matches_cpp_pipeline by replacing the absolute- inlier-count assertion with a corner-reprojection-error metric that's intrinsically invariant to BHC tree-topology cross-language nondeterminism (M9 #146 R1). Pre-brainstorm finding (closes #152 R3 by inspection rather than implementation): the C++ `kpm_query` `pose_out[12]` parameter is actually the 3x3 row-major homography in `pose_out[0..9]` with three trailing zeros for FFI convenience — same object Rust's `matched_geometry()` returns, not a 3x4 pose. See `kpm_c_api.cpp:156-166`. This eliminates the need for any new FFI shim; the existing kpm_query already exposes everything we need. The diagnostic trail (now complete) ----------------------------------- The original M9-1 parity assertion `|rust - cpp| <= 5 inliers` failed at `rust=441 cpp=456 (diff=15)` and stayed there across three PRs: * #145 (M9-1): introduced the gate, observed 15-inlier divergence * #149 (#146): BHC architecture (build-once + max_nodes_to_pop) — diff unchanged * #151 (#150): autoAdjustXYNumBins port — diff unchanged Both #149 and #151 shipped dedicated dual-mode FFI tests proving the Rust algorithmic ports byte-equivalent to C++ at the unit level (BHC partition + auto-adjust both pass byte-equivalence across 90 combined seeded random trials). The pipeline math is correct. The residual gap is BHC tree-topology cross-language nondeterminism: both Rust (`BTreeMap`/`HashMap`) and C++ (`std::unordered_map`) use unordered-key maps when grouping K-medoids assignments into child clusters during BHC build (binary_hierarchical_clustering.h:217). Hash orderings differ across toolchains — BHC trees differ — matches differ — downstream metrics differ by a stable ~15 inliers. The BHC algorithm tolerates this (priority-queue traversal handles ties), but byte-equivalent cross-language tree-build determinism isn't achievable without patching the WebARKit C++ source. Rather than chase upstream changes, this PR redefines the metric to one that's intrinsically invariant to the variance. Changes ------- * visual_database.rs (+116/-61): added a private `reproject_corners` helper inside the test module (YAGNI-correct — only caller is the parity test; promote later if M9-2 needs it). Rewrote test_visual_database_matches_cpp_pipeline to: - Extract Rust H via db.matched_geometry(). - Extract C++ H via the existing kpm_query's pose_out[0..9]. - Project the 4 reference corners through both homographies. - Compute per-corner Euclidean displacement; assert max <= 2.0 px. - arlog_i! the per-corner values for future tightening visibility. Removed the #[ignore] annotation — the test now runs by default. * docs/design/m9-parity-metric.md (NEW, ~330 lines): full brainstorming output (Understanding Summary, diagnostic trail, C++ pose_out layout finding, 10 decisions with alternatives + rationale, 4 assumptions, 3 risks with post-implementation status, files modified estimate, verification workflow, exit criteria, §10.5 measured outcome with the actual numbers from the first run, §10.6 milestone implications). Measured outcome (now baked into the design doc) ------------------------------------------------ First run on the pinball pair: max_displacement = 0.237754 px per corner: tl=0.109074, tr=0.237754, br=0.068354, bl=0.055763 Sub-pixel parity. Even with the 15-inlier divergence in matches, RANSAC converges to essentially the same homography because the matches are drawn from the same underlying images. Tolerance set per M9 #146 Decision 10 (max(2.0, ceil(observed))): const TOLERANCE_PX: f32 = 2.0; This is 8.4× the observed value — substantial safety margin against float-rounding drift in upstream M6-M8 components, hardware/toolchain rounding variation, and small RANSAC-seed-induced drift from future work. Risks materialized / did not materialize ---------------------------------------- * R1 (observed > 5 px ceiling) — did not materialize. Observed 0.24 px. * R2 (tolerance brittle) — mitigated by 8.4× margin from the 2.0 px floor. * R3 (C++ homography layout surprise) — did not materialize. A1 verified by source inspection (`kpm_c_api.cpp:156-166`); the pose_out layout is exactly as documented. Verification (CLAUDE.md §5) --------------------------- cargo fmt --all -- --check clean cargo build --all-features clean cargo clippy --all-targets --all-features 0 new warnings in modified files cargo test --lib --offline 407 passed, 2 ignored cargo test --features dual-mode --lib --offline 432 passed, 3 ignored (+1 active test = the un-#[ignore]'d parity gate) What this means for the M9 milestone ------------------------------------ The M9 dual-mode parity gate is closed. The test runs by default and asserts sub-pixel agreement between Rust and C++ homographies on the pinball pair. The heads-up posted to #141 (M9-2) recommends adopting the same corner-reprojection metric there instead of the current "zero divergence" framing. With this PR landed, M9-2 has a clear runway: land RustFreakMatcher + DualFreakMatcher, write its milestone gate using this metric pattern, then M9-3 flips the default off ffi-backend and Milestone 9 closes. Refs: #139 (M9 parent), #140/#145 (M9-1 baseline), #146/#149 (BHC R1 origin), #150/#151 (auto-adjust diagnostic). Closes #152 — corner reprojection metric defined, implemented, and verified passing on the pinball pair with sub-pixel agreement. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

This was referenced May 20, 2026

feat(kpm): redefine M9 dual-mode parity gate using pose accuracy + inlier ratio instead of absolute inlier count #152

Closed

feat(kpm): M9-2 - implement RustFreakMatcher and DualFreakMatcher #141

Open

kalwalt self-assigned this May 20, 2026

kalwalt added enhancement New feature or request rust code kpm FREAK labels May 20, 2026

kalwalt added this to Plan to port KPM to rust May 20, 2026

github-project-automation Bot moved this to Backlog in Plan to port KPM to rust May 20, 2026

kalwalt moved this from Backlog to In review in Plan to port KPM to rust May 20, 2026

kalwalt added this to the Milestone 9 — VisualDatabase & pure Rust backend milestone May 20, 2026

kalwalt merged commit e32cd93 into feat/freak-visual-database May 20, 2026
16 checks passed

github-project-automation Bot moved this from In review to Done in Plan to port KPM to rust May 20, 2026

kalwalt mentioned this pull request May 20, 2026

test(kpm): redefine M9 parity gate as corner reprojection (#152) #153

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(kpm): port HoughSimilarityVoting::autoAdjustXYNumBins (#150)#151

feat(kpm): port HoughSimilarityVoting::autoAdjustXYNumBins (#150)#151
kalwalt merged 1 commit into
feat/freak-visual-databasefrom
feat/hough-auto-adjust-xy-bins

kalwalt commented May 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

kalwalt commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Detailed Description

Pre-brainstorm finding

Two-layer dual-mode test diagnostics (both passing byte-equivalently)

What changed, by file

Review Checklist

Algorithm parity

API hygiene

Tests

Documentation

Risk Assessment (post-implementation)

Test Coverage

Visual Aid: pipeline before/after

Size & Splitting Notes

Review Automation Findings

What this PR ruled out — diagnostic value

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kalwalt commented May 20, 2026 •

edited

Loading