Cap erode() parameters and run memory guard on every backend (#1275)#1277
Open
brendancol wants to merge 1 commit intomainfrom
Open
Cap erode() parameters and run memory guard on every backend (#1275)#1277brendancol wants to merge 1 commit intomainfrom
brendancol wants to merge 1 commit intomainfrom
Conversation
- Cap `iterations`, `params['radius']`, and `params['max_lifetime']` via `_validate_scalar(max_val=...)` so `erode()` rejects pathological values before any allocation runs. - Rewrite `_check_erosion_memory` to include the `random_pos` buffer and brush working set in its budget, and wire it into `_erode_numpy` and `_erode_cupy` so every backend hits the guard. The dask paths inherit the check through their inner numpy/cupy calls. - Add 13 tests covering each cap, the memory guard on numpy and cupy, and the random_pos-dominated case. Mirrors the diffuse fix in #1268. Cat 3 NaN propagation and Cat 6 missing `_validate_raster` are deferred to separate PRs and tracked in the issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1275.
Summary
iterations,params['radius'], andparams['max_lifetime']at the publicerode()entry via_validate_scalar(max_val=...). New constants:_MAX_ITERATIONS=1e8,_MAX_RADIUS=1024,_MAX_LIFETIME=1e5. Out-of-range values raiseValueErrorbefore any allocation runs._check_erosion_memoryto include therandom_posbuffer and brush arrays in its budget (the old version only counted the raster). Wires it into_erode_numpyand_erode_cupyso the numpy and cupy paths get the guard too, not just dask.Test plan
iterations=0,iterations>cap,iterations=10**12, non-intiterationsraiseValueError/TypeErrorbefore any allocationradius=0,radius>capraiseValueErrormax_lifetime=0,max_lifetime>capraiseValueError_available_memory_bytesmakes the numpy and cupy paths raiseMemoryError(dask is no longer the only guarded backend)iterationson a tiny raster raisesMemoryErrormentioningrandom_pos(the buffer dominates)Deferred follow-ups (per the security-sweep one-fix-per-PR policy)
_erode_cpuor_erode_gpu_kernel. A NaN cell propagates through bilinear interpolation intodir_x/dir_y, NaN bounds checks fall through, and particles can deposit NaN viacuda.atomic.addinto arbitrary cells of the heightmap.erode()does not call_validate_raster(). Non-numeric or wrong-ndim input fails inside numba/cupy with a confusing error.