NDR mistakenly uses nodata value (-1) in eff calculation

[emlys]

It seems that the NDR eff calcuation mistakenly uses the effective retention nodata value (-1) as a real value on pour point pixels.

The processing stack is initialized with pour point pixels (that drain outside of the raster or drain to a nodata pixel). When these pixels are processed, we iterate over their downslope neighbors. Neighbors that are outside of the raster are correctly skipped:

invest/src/natcap/invest/ndr/effective_retention.h

Lines 176 to 179 in f9c143d

	if (neighbor.x < 0 or neighbor.x >= n_cols or
	neighbor.y < 0 or neighbor.y >= n_rows) {
	continue;
	}

but nodata neighbors are not skipped. I confirmed that at this point in the code, neighbor_effective_retention is -1 (the nodata value) and gets used as if it were real data in case 2:

invest/src/natcap/invest/ndr/effective_retention.h

Lines 192 to 204 in f9c143d

	neighbor_effective_retention = (
	effective_retention_raster.get(
	neighbor.x, neighbor.y));
	// Case 1: downslope neighbor is a stream pixel
	if (neighbor_effective_retention == STREAM_EFFECTIVE_RETENTION) {
	intermediate_retention = (
	retention_eff_lulc * (1 - current_step_factor));
	// Case 2: the current LULC's retention exceeds the neighbor's retention.
	} else if (retention_eff_lulc > neighbor_effective_retention) {
	intermediate_retention = (
	(neighbor_effective_retention * current_step_factor) +
	(retention_eff_lulc * (1 - current_step_factor)));

I think that the correct behavior would be to skip nodata neighbors (effectively treating them as 0 in the working_retention_eff sum).

The impact of this bug on final model results should be pretty minor, since it only affects the pour point pixels.

I am linking to the routing refactor branch because that's what I'm most familiar with right now, but the same problem exists in the original code.