Skip to content

Hash gpu_rtx mesh data without a full device-to-host copy; launch triangulation kernel once (#3691)#3693

Open
brendancol wants to merge 2 commits into
mainfrom
issue-3691
Open

Hash gpu_rtx mesh data without a full device-to-host copy; launch triangulation kernel once (#3691)#3693
brendancol wants to merge 2 commits into
mainfrom
issue-3691

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #3691

Two fixes on the create_triangulation() path shared by hillshade(shadows=True) and viewshed() on cupy-backed rasters:

  • The mesh cache hash no longer copies the whole raster to the host. hash(str(raster.data.get())) transferred the full array over PCIe (27 ms for a 4000x4000 float32 raster) and then hashed only numpy's truncated repr, i.e. corner and edge values. The new _data_hash() hashes the shape plus the exact bytes of the border rows/columns and a strided interior sample: 0.14 ms, and it now also detects interior-only changes the old hash was blind to. The border stays in the sample so the corner perturbation benchmarks/benchmarks/common.py uses to defeat mesh caching keeps working.
  • _triangulate_terrain now launches the triangulation kernel once instead of batching it into 100-block launches. The old loop turned a 4000x4000 raster into 157 serialized, under-occupied launches (38.2 ms vs 1.48 ms for one launch) and triggered a NumbaPerformanceWarning about grid size 100 on every call. CUDA's 1D grid limit is 2^31-1 blocks, so one launch covers any raster the _memory.py guard admits, and the kernel already bounds-checks global_id < W*H.

Measured end to end on an RTX A6000 (OptiX 9.1.0, 4000x4000 float32): hillshade_rtx 288 -> 211 ms, viewshed_gpu 278 -> 196 ms. Output is unchanged: hillshade and viewshed results on the same scene are identical before and after (verified numerically against main), and the rtx-vs-CPU parity tests pass.

Backends: only the cupy RTX/OptiX path is touched. numpy, dask+numpy, and dask+cupy paths are unaffected (gpu_rtx has no dask backend).

Also includes:

  • New asv benchmark benchmarks/benchmarks/gpu_rtx_mesh.py::CreateTriangulation timing the mesh build (hash, triangulation kernel, BVH) so a regression on this path shows up in benchmark runs.
  • New tests in test_gpu_rtx_mesh.py: _data_hash determinism, corner/interior/shape sensitivity, and a 400x300 triangulation test that exceeds the old 102,400-thread batch so single-launch grid coverage is exercised.
  • Sweep state CSV row for gpu_rtx (performance sweep bookkeeping).

Test plan:

  • pytest xrspatial/tests/test_gpu_rtx_mesh.py (14 passed, includes the 5 new tests, run on CUDA + rtxpy hardware)
  • pytest xrspatial/tests/test_hillshade.py (56 passed, includes test_hillshade_rtx_with_shadows)
  • pytest xrspatial/tests/test_viewshed.py (120 passed, 1 skipped, includes the rtx GPU paths)
  • pytest xrspatial/tests/test_gpu_rtx_memory.py xrspatial/tests/test_gpu_rtx_has_rtx.py xrspatial/tests/test_visibility.py (50 passed)
  • Before/after timing and output-equality comparison against main on the same GPU

@brendancol brendancol added gpu CuPy / CUDA GPU support performance PR touches performance-sensitive code labels Jul 23, 2026

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Hash gpu_rtx mesh data without a full device-to-host copy; launch triangulation kernel once (#3691)

Reviewed the full files, not just the diff, and ran the changed paths on a CUDA + rtxpy host (RTX A6000, OptiX 9.1.0).

Blockers (must fix before merge)

None found.

Suggestions (should fix, not blocking)

  • _data_hash (xrspatial/gpu_rtx/mesh_utils.py:48-49) hashes (data.shape, sample.get().tobytes()) but not the dtype. Two rasters with the same shape and identical sampled bytes but different dtypes (say float32 vs int32 reinterpretations) would collide. Within xrspatial this cannot bite because both entry points build a fresh RTX() whose getHash() never matches, but an external caller reusing an RTX instance across rasters would rely on this hash. Adding str(data.dtype) to the tuple costs nothing.

Nits (optional improvements)

  • hash() on bytes is salted per process (PYTHONHASHSEED), so datahash is not stable across runs. The old hash(str(...)) had the same property and the geometry cache lives in device memory, so nothing changes in practice; a zlib.crc32 or hashlib.blake2b digest would make the value deterministic if the cache ever outlives the process.
  • In _data_hash the border slices and the strided interior sample overlap (row 0 and column 0 appear twice, corners three times). Harmless for hashing, just redundant bytes in the sample.

What looks good

  • The single-launch rewrite is behavior-preserving: the kernel already guards global_id < W*H, and 2^31-1 blocks covers any raster the _memory.py guard admits. Verified hillshade and viewshed outputs on the same 1000x1000 scene are numerically identical to main.
  • Measured wins are real and reproduced locally: the hash drops from 27 ms to 0.14 ms on a 4000x4000 float32 raster, the triangulation phase from 38.2 ms to 1.5 ms, and end to end hillshade_rtx goes 288 -> 211 ms, viewshed_gpu 278 -> 196 ms. The NumbaPerformanceWarning about grid size 100 is gone (confirmed with warnings-as-errors).
  • The new tests pin the two behaviors that could silently regress: corner sensitivity of the hash (which benchmarks/benchmarks/common.py relies on to defeat mesh caching via z[-1, -1]) and full-grid coverage past the old 102,400-thread batch boundary (400x300 test with NaN/-1 sentinel prefill).
  • The two tests that previously reproduced the old hash formula to mock a cache hit now call _data_hash instead of duplicating the expression.
  • The new CreateTriangulation asv benchmark isolates the mesh build (hash + kernel + BVH) and skips cleanly via NotImplementedError when rtxpy is absent, matching the existing rtxpy benchmark convention.
  • State CSV row is a clean one-line CRLF-preserving insertion.

Checklist

  • Algorithm matches reference/paper (n/a; mesh geometry unchanged, outputs verified identical to main)
  • All implemented backends produce consistent results (cupy/RTX path only; numpy triangulation path untouched; gpu_rtx has no dask backend)
  • NaN handling is correct (all-NaN rasters still rejected by the pre-existing maxH guard before hashing)
  • Edge cases are covered by tests (corner/interior/shape hash sensitivity, grid larger than the old batch, existing 4x3 and guard tests still pass)
  • Dask chunk boundaries handled correctly (n/a, no dask path)
  • No premature materialization or unnecessary copies (the full-raster D2H copy is what this PR removes; only a perimeter + strided sample crosses to the host now)
  • Benchmark exists (new benchmarks/benchmarks/gpu_rtx_mesh.py::CreateTriangulation)
  • README feature matrix updated (n/a, no new public function, no backend change)
  • Docstrings present and accurate (_data_hash docstring explains the sampling and the benchmark corner contract)

Test runs on this host: test_gpu_rtx_mesh.py 14 passed, test_hillshade.py 56 passed, test_viewshed.py 120 passed / 1 skipped, test_gpu_rtx_memory.py + test_gpu_rtx_has_rtx.py + test_visibility.py 50 passed. flake8 clean on the changed files (the isort disagreement in test_gpu_rtx_mesh.py predates this PR).

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review (follow-up): commit 867e101

Re-reviewed after the review follow-up commit. Scope of the delta: _data_hash now feeds shape, dtype, and the sample bytes through hashlib.blake2b(digest_size=8) instead of the salted built-in hash(), plus two new tests.

Blockers (must fix before merge)

None found.

Suggestions (should fix, not blocking)

None.

Nits (optional improvements)

None. The overlapping-sample nit from the first review was dismissed with a reason I agree with: deduplicating the border/interior slices would add 1-row/1-column edge cases for no measurable gain.

Verification of the delta

  • Both first-review findings are addressed: dtype now participates in the digest (Suggestion), and the value is deterministic across processes (Nit about PYTHONHASHSEED salting). One change resolves both.
  • digest_size=8 yields exactly 8 bytes, so int.from_bytes(..., 'little') always fits np.uint64; the NumPy 2.0 out-of-bounds cast issue that #805 fixed cannot recur here.
  • The dtype test uses all-zero float32 vs int32 arrays, which are byte-identical, so it fails unless dtype is in the digest. The determinism test pins a golden value; I confirmed the same value is produced under two different PYTHONHASHSEED settings.
  • Timing holds: _data_hash on a 4000x4000 float32 raster is 0.24 ms with blake2b (0.14 ms with the salted hash, 27 ms before this PR). No effect on the end-to-end numbers in the PR body.
  • Full mesh suite passes (16 tests), hillshade + viewshed suites pass (176 passed, 1 skipped), flake8 clean on changed files.

Checklist

  • Algorithm matches reference/paper (n/a; hash formula change only, mesh geometry untouched)
  • All implemented backends produce consistent results (cupy/RTX path only)
  • NaN handling is correct (unchanged; all-NaN rasters still rejected before hashing)
  • Edge cases are covered by tests (dtype collision and cross-process determinism added)
  • Dask chunk boundaries handled correctly (n/a)
  • No premature materialization or unnecessary copies (sample-only D2H unchanged)
  • Benchmark exists (CreateTriangulation, unchanged by this commit)
  • README feature matrix updated (n/a)
  • Docstrings present and accurate (docstring updated to describe the blake2b digest)

Nothing further; ready from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gpu CuPy / CUDA GPU support performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gpu_rtx: create_triangulation copies the full raster to host for hashing and splits the mesh kernel into 157 launches

1 participant