Skip to content

hydro: cellsize not validated before division (silent miscompute) #1429

@brendancol

Description

@brendancol

Description

twi_d8, flow_direction_d8 / _dinf / _mfd, and flow_length_d8 / _dinf / _mfd all call get_dataarray_resolution() and then divide by the result without checking whether the cellsize is finite and positive.

When a DataArray has 1-D coordinates (or coordinates that resolve to zero or non-finite spacing), get_dataarray_resolution can return 0.0 or NaN. Division by zero in the JIT kernels then produces:

  • flow_direction_d8 / _dinf / _mfd: (center - v) / cellsize -> inf for all neighbors. max_slope is unhelpfully inf, every direction looks equally steep, and the chosen flow direction is undefined.
  • twi_d8: cellsize = cx * cy = 0 -> sca = fa * 0 -> log(0 / _TAN_MIN) = -inf everywhere.
  • flow_length_*: distance accumulator is multiplied by 0, output is silently zero everywhere downstream.

None of these raise; the caller gets a finite-looking-but-wrong raster.

Expected behavior

Each of these public functions checks cellsize_x and cellsize_y are finite and positive immediately after get_dataarray_resolution() and raises ValueError if not.

Proposed fix

Three-line guard at each site:

cellsize_x, cellsize_y = get_dataarray_resolution(<arg>)
if not (np.isfinite(cellsize_x) and cellsize_x > 0
        and np.isfinite(cellsize_y) and cellsize_y > 0):
    raise ValueError(
        f"<func_name>(): cellsize must be a positive finite number "
        f"(got {cellsize_x}, {cellsize_y}).  Ensure the input DataArray "
        f"has at least 2 cells in each spatial dimension and finite coords."
    )

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinginput-validationInput validation and error messages

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions