natcap / invest

InVEST®: models that map and value the goods and services from nature that sustain and fulfill human life.
Apache License 2.0
168 stars 69 forks source link

UFRM: incorrect ValueError raised if biophysical table includes a row of all 0s #1123

Closed davemfish closed 1 year ago

davemfish commented 1 year ago

This came up on this forum post.

As is somewhat common, a LULC raster without a nodata value was used. But the raster was filled with 0s that probably represent nodata (or maybe water), and so the biophysical table was given a row for lucode 0, filled with 0s for CN_A, CN_B, CN_C, CN_D.

I don't know the details of this model, and whether setting those parameters to 0 is equivalent to setting that pixel value to nodata, but I don't think it matters.

Here's the problem

This model does fancy things using a sparse matrix as a lookup table for these CN values. Such a matrix can appear to have a row full of 0s for two different reasons:

  1. Because the biophysical table had a row full of 0s (this should not raise an error, but does)
  2. Because a landuse code was missing from the biophysical table (this should raise an error, and does)
davemfish commented 1 year ago

Some context for this; this bug was created while fixing another one. #686 #698

dcdenu4 commented 1 year ago

This came up again on the forums and I was writing a new issue when I discovered this one existed. Here's what I was writing:

I think we've mostly settled on being able to support raster inputs that do not have a defined nodata value because GDAL supports it. However, a lot of rasters that do not have a defined nodata often have a value "acting" as one. So, in a lot of our models this means adding a row to a biophysical table that accounts for that value which is "acting" as nodata but not defined as such. In the UFM model there is a situation where the model checks to make sure there aren't any LULC values not represented in the table as follows:

 # Even without an IndexError, still must guard against           
 # lucodes that can index into the sparse matrix but were         
 # missing from the biophysical table. They have rows of all 0.   
 if not cn_matrix.sum(1).all():                                   
     empty_rows = numpy.where(lucode_to_cn_table.sum(1) == 0)     
     missing_codes = numpy.intersect1d(valid_lucodes, empty_rows) 
     raise ValueError(                                            
         f'The biophysical table is missing a row for lucode(s) ' 
         f'{missing_codes.tolist()}')                             

This logic doesn't allow for a user entered row of all 0 values, which end up being caught as that rows lucode not being represented in the table, when it actually is.

Forum link: https://community.naturalcapitalproject.org/t/urban-flood-risk-mitigation-model-area-of-analysis/3583/8