Add PointCloudHashGrid: uniform-grid counterpart to PointCloudBVH

[rmrsk]

Summary

Adds PointCloudHashGrid<T, Meta>, a uniform-grid alternative to PointCloudBVH that answers the
same nearest-neighbor / closest-point queries through the identical interface (drop-in
interchangeable), and is faster to build and query on near-uniform clouds. Ships with its own
example and tests, and restructures the point-cloud documentation into a concepts page and an
implementation page covering both backends.

Closes #107.

Background

PointCloudBVH (merged in #106) gives turnkey nearest-neighbor / closest-point queries over a point
cloud, backed by a BVH. While finishing that work we prototyped a uniform spatial grid as an
alternative that circumvents the tree entirely, and found it clearly faster to build on near-uniform
clouds. Issue #107 proposed promoting that prototype into a first-class class mirroring
PointCloudBVH. This PR does so, on top of main now that #106 has merged.

Solution

Adds PointCloudHashGrid<T, Meta>, which answers the same queries as PointCloudBVH and exposes
the same public interface — same Hit, same closestPoint / closestPoints / nearestNeighbor
/ nearestNeighbors / allNearestNeighbors, same O(N) brute-force reference queries, same
position() / metadata() / numPoints() accessors — so the two are drop-in interchangeable.

• Build: points are counting-sorted into a dense CSR bucket array keyed by integer cell
  coordinates (O(N), no tree, no recursion). Cell size is derived from a target average occupancy
  (~1 point/cell by default).
• Query: an expanding-shell search outward from the query point's cell with an exact stopping
  rule — after searching Chebyshev radius R, the world region within the covered cell faces is fully
  searched, so once the k-th best distance is inside that bound no neighbor can be missed. Handles
  external queries and queries outside the grid box (clamped cell); k > 1 carries a sorted k-best
  set through the shells; self-queries exclude the point itself.

On a near-uniform 500k cloud the grid builds and queries fast (see Examples/NearestNeighborHashGrid
vs Examples/NearestNeighborBVH for measured numbers on your machine).

Included alongside the class:

• EBGeometry.hpp include and Tests/InstantiateAll.cpp coverage (both precisions).
• TestPointCloudHashGrid — checked against an independent brute-force oracle in both precisions,
  including the outside-the-box query case.
• Examples/NearestNeighborHashGrid.
• Documentation restructured to mirror the BVH/SFC concept-vs-implementation split: PointCloud.rst
  becomes a Concepts page (point-cloud proximity queries; tree vs uniform-grid acceleration), and
  a new ImplemPointCloud.rst is the Implementation page with two sections, PointCloudBVH and
  PointCloudHashGrid. Plus full doxygen.

The class follows the same conventions as PointCloudBVH: JavaDoc doc style, point terminology, and
EBGEOMETRY_EXPECT / static_assert debug hooks (finite coordinates, index bounds, buffer / a_k
validity, per-cell invariants).

Side-effects

• The class is not a BVH: unlike PointCloudBVH it cannot be composed as a primitive inside an
  outer BVH/CSG union. It serves only point queries.
• The grid is bounded-domain: dense cells sized to the cloud bounding box, O(bboxVolume / h³)
  memory — fine for a compact cloud, wasteful for one sparse but spread over a large box.
• The performance profile is density-dependent: a single global cell size only suits near-uniform
  clouds; for clustered / multi-scale clouds PointCloudBVH's density adaptivity is the better choice
  — documented in the class comments, PointCloud.rst / ImplemPointCloud.rst, and the example README
  so callers can pick.
• Docs move: the point-cloud documentation added in PointCloudSoA: SoA point primitives, PointCloudBVH, direct PackedBVH builders + StoragePolicy #106 (PointCloud.rst, which documented
  PointCloudBVH) is restructured — PointCloud.rst is now conceptual and the implementation prose
  moves to the new ImplemPointCloud.rst. The Chap:ImplemPointCloud label moves with it to the new
  page (existing cross-references stay valid); a new Chap:PointCloud label names the concepts page.

Alternative solutions

• Shared abstract base / concept for PointCloudBVH and PointCloudHashGrid so callers can
  select a structure polymorphically. Deferred — the two share an interface by convention here; a
  formal shared interface is a separate refactor (noted in Add a PointCloudHashGrid class: uniform-grid counterpart to PointCloudBVH for nearest-neighbor queries #107).
• True spatial hashing (unordered_map<cellId, bucket>) instead of a dense array, to bound memory
  to O(N) on unbounded/sparse domains. Deferred; the dense grid is the fast bounded-domain case and
  the natural first cut. The same query logic would apply.
• A k=1 fast path mirroring the BVH's seeded single-nearest specialization — not needed yet; the
  general k-best path is already fast for k=1.

Reviewer checklist (to be completed by a human)

• The test suite compiles and runs to completion without warnings or errors.
• All relevant new features are documented in the user documentation (Sphinx).
• This contribution does not break existing sections in the user documentation.
• All relevant APIs are documented in the doxygen documentation.
• Appropriate labels have been assigned to this PR.
• New or revised proper licensing and copyright information is in place.
• A PR review has been run using @claude review.
• The continuous integration and testing hooks at GitHub run to completion.

[rmrsk]

@claude review

[github-actions]

Claude finished @rmrsk's task in 4m 54s —— View job

---

Claude's Review

Tasks

• Gather context (PR description, changed files list)
• Read new/changed source files (PointCloudHashGrid.hpp, PointCloudHashGridImplem.hpp)
• Read tests (TestPointCloudHashGrid.cpp) and example
• Check docs updates (PointCloud.rst, ImplemPointCloud.rst)
• Check build-system wiring (EBGeometry.hpp, CMakeLists, InstantiateAll.cpp)
• Post review feedback

Overall this is a clean, well-designed addition. The counting-sort build, the CSR bucket layout, and the expanding-shell query with the exact Chebyshev stopping bound are all correct as far as I traced them (including the clamped-cell handling for out-of-box queries, and the "hollow shell" cell-enumeration trick, which I hand-verified against several boundary-clamped cases). The public interface genuinely mirrors PointCloudBVH::Hit / method set, docs are wired up and consistent (PointCloud.rst concepts page + ImplemPointCloud.rst implementation page, both linked from index.rst), and EBGeometry.hpp / Tests/InstantiateAll.cpp / CMakeLists are all updated. Whitespace conventions (blank line before/after every loop) are followed throughout the new implementation file.

One correctness/robustness concern worth a look before merge, plus a minor test-coverage gap:

1. Unbounded/UB grid-dimension computation for non-uniform bounding boxes

Source/EBGeometry_PointCloudHashGridImplem.hpp:59-81

m_h = std::cbrt(vol / T(numPoints) * a_targetPerCell);
...
m_nx = std::max(1, int((hi[0] - m_lo[0]) * m_invH) + 1);

m_h is derived from average density over the whole bounding box. A cloud that is mostly tightly clustered plus even one distant outlier point gives a bounding box far larger than the cluster, but a target-density m_h sized to the cluster's tiny local spacing. (hi[0]-m_lo[0]) * m_invH can then be huge — large enough that int(...) on a double/float exceeding INT_MAX is undefined behavior (not just wrap-around), and even where it doesn't overflow, m_nx*m_ny*m_nz (line 83) can reach billions of cells, causing an unconditional m_cellStart.assign(nCells + 1, 0) allocation attempt (bad_alloc/crash) with no fallback or cap.

The PR description's "Side-effects" section documents the general wasteful-memory case for a sparse cloud spread over a large box, but this is a sharper failure mode: a single outlier can turn an otherwise-uniform, well-behaved cloud into a crash, and there's currently no EBGEOMETRY_EXPECT/clamp guarding it (contrast with the existing degenerate-volume fallback at lines 64-75, which does handle the zero-volume case gracefully). Worth considering either a sanity cap on nCells (falling back to a coarser m_h, similar to the existing degenerate-box fallback) or at least a debug-mode assertion so this fails loudly instead of an unbounded allocation attempt.

Fix this →

2. No test coverage for degenerate/non-uniform clouds

Tests/TestPointCloudHashGrid.cpp

The test suite (like TestPointCloudBVH.cpp, which it mirrors) only exercises a uniform-random cloud in the unit cube. The constructor has distinct handling for degenerate bounding boxes (zero volume, lines 64-75) and, per point 1 above, is sensitive to non-uniform/clustered distributions — none of that is currently exercised by a test (empty cloud, single point, all-coincident points, or a small clustered-plus-outlier cloud). Since this class's own docs call out clustered clouds as its weak case, a small targeted test would both cover the degenerate-fallback branch and pin down the intended (non-crashing) behavior for the outlier scenario in point 1.

Minor

• m_cellStart/m_cellPoints use std::uint32_t, capping numPoints and nCells under ~4.29 billion with no assertion; almost certainly fine in practice, just noting it's unguarded.

Nothing else stood out — the query logic, self-query exclusion, brute-force references, and doc cross-references all look solid.

[rmrsk]

@claude review

[github-actions]

Claude finished @rmrsk's task in 3m 40s —— View job

---

Claude's Review

Tasks

• Gather context (PR description, changed files list, all 9 commits vs origin/main)
• Read new/changed source files (PointCloudHashGrid.hpp, PointCloudHashGridImplem.hpp)
• Read tests (TestPointCloudHashGrid.cpp) and example
• Check docs updates (PointCloud.rst, ImplemPointCloud.rst)
• Check build-system wiring (EBGeometry.hpp, CMakeLists, InstantiateAll.cpp)
• Post review feedback

This PR has evolved since the last @claude review pass (commit a0280ea, "harden the stopping rule, cap grid size, add edge-case tests") specifically to address that review's feedback. Re-checked the current state:

• Test coverage for degenerate clouds — now covered by the new "PointCloudHashGrid edge cases" section in Tests/TestPointCloudHashGrid.cpp (empty cloud, single point, coincident points, planar/zero-volume cloud, k > cloud size). Resolved.
• Unguarded uint32_t cap — now has an explicit EBGEOMETRY_EXPECT(numPoints <= ... uint32_t::max()) (EBGeometry_PointCloudHashGridImplem.hpp:42). Resolved.
• Exact stopping-rule ULP risk — the m_h vs m_invH face-reconstruction fix (pulling each guaranteed-covered face one whole cell inward, evaluated only for r >= 1) is correct: it only makes the bound more conservative, so it can cost at most one extra shell and cannot cause an incorrect early stop. Traced through the boundary conditions (c - r > 0 / c + r < nAxis - 1) and they correctly identify "unsearched cells remain on this side."

Remaining concern: the grid-size cap doesn't bound cell count for an anisotropic (clustered + outlier) cloud

Source/EBGeometry_PointCloudHashGridImplem.hpp:81-93

The new cap only raises m_h when it is smaller than hMin = cbrt(vol / maxCells), where vol is the full bounding-box volume. This works for the case the commit message targets (pathologically small a_targetPerCell on a roughly-uniform/cubic cloud), but it does not bound nx * ny * nz when the bounding box is highly anisotropic — which is exactly the "tight cluster + one distant outlier" scenario flagged in the previous review, and it isn't fixed.

Worked example: 999 points uniform in [0,1]³ plus one outlier at (D, 0.5, 0.5):

• ext ≈ (D, 1, 1), so vol ≈ D.
• m_h_initial = cbrt(vol/N) = cbrt(D/1000) ∝ D^(1/3).
• hMin = cbrt(vol/maxCells) = cbrt(D/8064), which scales with the same power of D — so the ratio m_h_initial / hMin = cbrt(maxCells/N) is a constant independent of D. The clamp never engages regardless of how large D is.
• Meanwhile ny = nz = 1 (the cluster's y/z extent is « h), but nx ≈ D / m_h_initial ∝ D^(2/3) — unbounded as D → ∞.

Concretely: D = 1e9 → nx ≈ 1e7 cells (~40 MB m_cellStart, wasteful but survives). D = 1e12 → nx ≈ 1e9 cells (4 GB m_cellStart — allocation failure/OOM in practice). Push D further (3e12 with this N) and int((hi[0]-m_lo[0]) * m_invH) overflows int range, which is UB, not just a big allocation — the same failure mode as originally flagged, and there's still no EBGEOMETRY_EXPECT upper-bounding nCells/m_nx/m_ny/m_nz to catch it even in debug builds. A single mis-scaled or sentinel-valued point (finite, so it passes the existing isfinite check, just numerically huge relative to the rest of the cloud) is a realistic way to hit this in practice.

None of the new "edge cases" or "extreme cell-size targets" tests exercise an anisotropic bounding box (they vary a_targetPerCell, not cloud shape), so nothing in the current suite would catch this.

Suggest capping directly on the computed nx/ny/nz (or the resulting nCells) after they're computed from the per-axis extents, with a fallback that coarsens m_h until the product fits the budget — rather than deriving the cap only from the isotropic vol-based m_h, which doesn't track actual per-axis cell counts for a non-cube-ish box.

Fix this →

Minor

• Everything else — the CSR build, expanding-shell query logic, self-query exclusion, brute-force references, EBGeometry.hpp/InstantiateAll.cpp/CMakeLists wiring, and the PointCloud.rst/ImplemPointCloud.rst docs split (no literalinclude, labels consistent, cross-references intact) — all look correct and consistent with PointCloudBVH's conventions.
  (branch: pointcloud-hashgrid)

[rmrsk]

@claude review

[github-actions]

Claude finished @rmrsk's task in 1m 46s —— View job

---

Claude's Review

Tasks

• Gather context (PR description, prior review rounds, changed files)
• Read the new commit (491f8ac, "bound the grid size for anisotropic boxes") that addresses the previous review's anisotropic-cap concern
• Re-verify the anisotropic grid-size cap logic against the worked example from the last review
• Check for new/updated test coverage for anisotropic clouds
• Post final review feedback

This round (491f8ac, on top of a0280ea) directly targets the one remaining concern from the previous review: the grid-size cap previously derived only from the isotropic vol-based m_h, so it never engaged for a highly anisotropic bounding box (tight cluster + one distant outlier).

The fix is correct. Source/EBGeometry_PointCloudHashGridImplem.hpp:93-129:

• predictedCells() (lines 96-102) now computes the actual per-axis dx/dy/dz from the real bounding-box extents in double, not the isotropic vol-derived approximation — so it correctly captures the case where two axes stay pinned at 1 cell while the outlier axis blows up.
• The coarsening loop (lines 104-113) repeatedly scales m_h by cbrt(predicted / maxCells). Because nCells scales as m_h⁻³ in the regime that matters (all axes shrinking together), one iteration lands very close to maxCells; the guard < 100 bound is a generous safety net, not something that's actually needed for convergence — I traced through the math and it converges in practice within 1-2 iterations.
• maxCells is explicitly capped at INT_MAX (line 94), and the final m_nx/m_ny/m_nz (lines 120-122) are derived from the same double-precision formula used in the cap check, so the EBGEOMETRY_EXPECT(nCells <= maxCells) (line 129) guarantees no individual axis dimension nor the product can overflow int — closing the UB path flagged previously, not just the allocation-size path.
• I re-ran the worked example from the last review (999 clustered points + 1 outlier at distance D) by hand against this new logic: since dy = dz = 1 regardless of D (the cluster's extent stays ≤ 1 while m_h only ever grows), the cap now bounds dx alone to ~maxCells, for any D — including the D ≳ 3e12 case that previously reached int overflow.

Test coverage: Tests/TestPointCloudHashGrid.cpp gained exactly the missing case — "clustered cloud with a distant outlier stays bounded and correct" (999 unit-cube points + 1 outlier at x = 1e6), asserting numPoints() and checking nearestNeighbor against the brute-force oracle for a sample of points. This is the right test: it exercises both "doesn't crash/overflow" and "queries stay exact after coarsening."

All previously-flagged items are now resolved:

1. Unbounded/UB grid-dimension computation — fixed by this commit.
2. No test coverage for degenerate/non-uniform clouds — covered since a0280ea, now extended with the anisotropic case.
3. Unguarded uint32_t cap — EBGEOMETRY_EXPECT added since a0280ea.

I wasn't able to actually configure/build/run ctest --preset debug in this session (the cmake/build Bash invocations required interactive approval that wasn't available in this automated run) — I verified the logic by static reading/hand-tracing rather than executing the test suite. Worth having CI (or a local ctest --preset debug) confirm the new test passes green before merge, but I have high confidence in the fix based on the code trace above.

Nothing else stood out — the rest of the class (CSR build, expanding-shell query, self-query exclusion, docs) was already reviewed as solid in the prior two passes and is unchanged here.