Skip to content

Validate hotspots() kernel like apply()/focal_stats()#2799

Merged
brendancol merged 2 commits into
mainfrom
issue-2771
Jun 1, 2026
Merged

Validate hotspots() kernel like apply()/focal_stats()#2799
brendancol merged 2 commits into
mainfrom
issue-2771

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

hotspots() skipped the kernel validation that apply() and focal_stats() run via custom_kernel(), so bad kernels produced confusing errors or wrong behavior:

  • Run custom_kernel(kernel) before the memory check. None and list-of-list kernels now raise a clear ValueError instead of AttributeError on kernel.shape, and even-dimensioned kernels are rejected for improper shape (previously they silently succeeded).
  • Add a zero-sum kernel guard. hotspots() is the only focal function that normalizes by kernel.sum(), so a zero-sum kernel previously divided by zero.

Backend coverage

The validation runs before backend dispatch, so it applies to numpy, cupy, dask+numpy, and dask+cupy.

Test plan

  • New regression tests for None, list-of-list, even-dim, and zero-sum kernels
  • Valid-kernel happy path still produces the expected classification
  • Full test_focal.py suite passes (155 passed)

Closes #2771

hotspots() never ran its kernel through custom_kernel(), so kernel=None
and a list-of-list kernel raised AttributeError on kernel.shape, an
even-dimensioned kernel silently succeeded, and a zero-sum kernel divided
by zero during normalization.

Run custom_kernel(kernel) before the memory check and add a zero-sum
guard (hotspots is the only focal function that normalizes by
kernel.sum()). Add regression tests for the None, list-of-list, even-dim,
and zero-sum cases plus a valid-kernel happy path.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jun 1, 2026
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

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

PR Review: Validate hotspots() kernel like apply()/focal_stats()

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

None.

Nits (optional improvements)

  • focal.py:1509 -- the guard catches exactly-zero sums. A mixed-sign kernel that sums to roughly zero but not exactly zero (floating-point near-cancellation) still slips through and produces a near-infinite normalization. This matches focal_stats() and is within the issue scope, so leaving it is fine.

What looks good

  • Validation runs before backend dispatch, so numpy, cupy, dask+numpy, and dask+cupy all reject bad kernels the same way. The 3D path recurses through hotspots() per band, so each band validates too.
  • custom_kernel() runs before kernel.sum(), so the sum call always has an ndarray. No AttributeError risk.
  • Error messages name the function and explain the failure.
  • Tests cover None, list-of-list, even-dim, and zero-sum kernels plus a valid happy path. Full test_focal.py passes (155).

Checklist

  • Algorithm unchanged; only input validation added
  • All backends get consistent validation (runs before dispatch)
  • NaN handling unchanged
  • Edge cases covered by tests
  • Dask path unaffected (validation is pre-dispatch)
  • No premature materialization
  • Benchmark not needed (validation-only change)
  • README feature matrix not applicable (no new function)
  • Docstrings present (existing hotspots docstring unchanged)

@brendancol brendancol merged commit 88e3e05 into main Jun 1, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hotspots() skips kernel validation: bad kernels raise AttributeError or divide by zero

1 participant