Undefined nodata values cause nodata checking to fail in numpy.isclose

[phargogh]

We have encountered a number of issues on the forums (here's a recent example) where the model is crashing during the comparison of a floating-point array with an undefined nodata value. In the model, it looks something like this:

valid_pixels = numpy.isclose(array, nodata_value)

But if nodata_value=None:

>>> import numpy
>>> array = numpy.array([1]))
>>> valid_pixels = numpy.isclose(array, None)
TypeError: ufunc 'isfinite' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''

Note that if we have an integer array, comparing with None is no problem at all:

>>> import numpy
>>> numpy.ones((1,1), dtype=numpy.uint8) == None
array([[ False]])

As documented in #226, having an unset nodata value is a valid configuration of a raster and should be supported throughout InVEST.

• Locate all cases where we're using numpy.isclose with a possibly-undefined nodata value
• Determine and implement a fix
• Ensure that we're covering this case in our tests

[phargogh]

Just saw another instance of this in https://community.naturalcapitalproject.org/t/habitat-quality-typeerror-ufunc-isfinite/1168/2

[richpsharp]

Hi @emlys! I'm going through old email and just saw this issue and draft pull request. I know this isn't the biggest deal, and I'm a little worried I don't entirely understand the issue, but if I do can I suggest a change to the proposed function name? Is this is a fix to pass through to numpy.is_close but guard against a None or some other invalid value? If so, could I propose that this function be called utils.is_close since that is the behavior it's mimicking? That way code will read the same and we won't have to guess what "valid" means or what the arguments are, etc? This would also save having to logically reverse any ~ operator in the code now or in a future maintenance issue where for some reason there are a lot of numpy.is_closes sitting around (numpy.is_close is easier to change to utils.is_close rather than ~utils.is_valid?)

I hope that's helpful, and in case I'm not understanding this issue at all, please feel free to disregard.

[emlys]

Hi @emlys! I'm going through old email and just saw this issue and draft pull request. I know this isn't the biggest deal, and I'm a little worried I don't entirely understand the issue, but if I do can I suggest a change to the proposed function name? Is this is a fix to pass through to numpy.is_close but guard against a None or some other invalid value? If so, could I propose that this function be called utils.is_close since that is the behavior it's mimicking? That way code will read the same and we won't have to guess what "valid" means or what the arguments are, etc? This would also save having to logically reverse any ~ operator in the code now or in a future maintenance issue where for some reason there are a lot of numpy.is_closes sitting around (numpy.is_close is easier to change to utils.is_close rather than ~utils.is_valid?)

I hope that's helpful, and in case I'm not understanding this issue at all, please feel free to disregard.

You're right that the new function is just essentially ~numpy.isclose with a check for the case where the input is None! The reason I chose the name is_valid is that almost everywhere, it's being used to make a valid mask for a numpy array showing which elements are not nodata. So almost everywhere, ~numpy.isclose is being replaced with utils.is_valid (for example https://github.com/natcap/invest/pull/269/files#diff-99cac412fe1d4f5e95c6ad79393c56d9L874). is_valid made more sense to me in the context where it's used, and also got rid of a lot of ~s. But I'm happy to change it if you still think that ~utils.is_close would be better!

[phargogh]

Since we merged #269 , I think this might be safe to close! Feel free to reopen if needed or if there ends up being more to resolve here.

[dcdenu4]

SDR might have a similar issue happening still as reported from a forum user: https://community.naturalcapitalproject.org/t/sdr-typeerror-isfinite/2491/2

The error message:

2022-02-11 14:22:47,373 (pygeoprocessing.geoprocessing) geoprocessing.raster_calculator(482) INFO 100.0% complete
2022-02-11 14:22:47,374 (pygeoprocessing.geoprocessing) geoprocessing.raster_calculator(485) INFO Waiting for raster stats worker result.
2022-02-11 14:23:05,485 (pygeoprocessing.routing.routing) Task._call(1093) INFO 0.0% complete
2022-02-11 14:23:16,000 (pygeoprocessing.routing.routing) Task._call(1093) INFO 40.4% complete
2022-02-11 14:23:27,000 (pygeoprocessing.routing.routing) Task._call(1093) INFO 90.1% complete
2022-02-11 14:23:35,767 (pygeoprocessing.routing.routing) Task._call(1093) INFO filter out incomplete divergent streams
2022-02-11 14:23:36,097 (pygeoprocessing.routing.routing) Task._call(1093) INFO 100.0% complete
2022-02-11 14:23:36,133 (taskgraph.Task) Task.add_task(706) ERROR Something went wrong when adding task calculate W (9), terminating taskgraph.
Traceback (most recent call last):
  File "taskgraph\Task.py", line 674, in add_task
  File "taskgraph\Task.py", line 1093, in _call
  File "natcap\invest\sdr\sdr.py", line 1006, in _calculate_w
  File "natcap\invest\utils.py", line 915, in reclassify_raster
  File "pygeoprocessing\geoprocessing.py", line 1853, in reclassify_raster
  File "<__array_function__ internals>", line 180, in isclose
  File "numpy\core\numeric.py", line 2358, in isclose
TypeError: ufunc 'isfinite' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''
2022-02-11 14:23:36,144 (natcap.invest.utils) utils.prepare_workspace(167) ERROR Exception while executing Sediment-Delivery-Ratio

Whole logfile
InVEST-Sediment-Delivery-Ratio-log-2022-02-11--13_54_19.txt

[davemfish]

SDR might have a similar issue happening still as reported from a forum user: https://community.naturalcapitalproject.org/t/sdr-typeerror-isfinite/2491/2

This traceback was actually the result of a incorrectly formatted table #890

So false alarm on the undefined nodata still being a problem.