Update to pygeoprocessing 2.4.2 #1437
Conversation
| @@ -298,17 +298,6 @@ | |||
| "filtered_[THREAT]_aligned.tif": { | |||
| "about": "Filtered threat raster", | |||
| "bands": {1: {"type": "ratio"}}, | |||
| }, | |||
| "kernels": { | |||
There was a problem hiding this comment.
This was not actually used anywhere.
There was a problem hiding this comment.
Ah, this looks like it's left over from the old convolution-based workflow we recently reverted. Thanks for catching this!
There was a problem hiding this comment.
Wow, thank you for this big update, @emlys ! It is so nice to see all of the array masking logic abstracted away to raster_map.
I just had two questions for you about how the model's behavior was changing in UCM and in Pollination that were resulting in updated regression values. It might save us some headaches (and forums questions!) down the road if we could identify and document in HISTORY what's changing that results in different outputs.
| args=(storage_path_list, file_registry[output_key]), | ||
| func=pygeoprocessing.raster_map, | ||
| kwargs=dict( | ||
| op=sum_op, |
There was a problem hiding this comment.
Aha, right! This would be a great place to use functools.partial, but we can't because it doesn't have a __name__. natcap/taskgraph#83
| @@ -298,17 +298,6 @@ | |||
| "filtered_[THREAT]_aligned.tif": { | |||
| "about": "Filtered threat raster", | |||
| "bands": {1: {"type": "ratio"}}, | |||
| }, | |||
| "kernels": { | |||
There was a problem hiding this comment.
Ah, this looks like it's left over from the old convolution-based workflow we recently reverted. Thanks for catching this!
| 'avd_eng_cn': 3520213.280928277, | ||
| 'avd_eng_cn': 3520217.313878, |
There was a problem hiding this comment.
Are these values changing because of a change in how we are masking? If the behavior is changing, I wonder if we should note that change in HISTORY
There was a problem hiding this comment.
These had to be updated because of changes to zonal_statistics. Out of almost 1500 building polygons in the test data, zonal stats results were different (and more correct) for 7 of them. I am guessing because we used to reproject the raster to match the vector; now we reproject the vector to match the raster, which should be more accurate. This probably only showed up in UCM because it uses so many polygons and they are small relative to the pixel size.
There was a problem hiding this comment.
I added a general note in history.
| self.assertAlmostEqual(result_sum, 58.669518, places=2) | ||
| self.assertAlmostEqual(result_sum, 58.407155, places=2) |
There was a problem hiding this comment.
Are these changing because of the change in how we're building kernels? Or is it more about masking? Either way, it might be useful to note in the HISTORY why values are changing slightly.
There was a problem hiding this comment.
This is because of the switch to pygeoprocessing.kernels, which is more correct in some cases. The kernel implementation in pollination didn't always produce symmetrical kernels:
With pygeoprocessing.kernels:
…m/emlys/invest into task/update-pygeoprocessing-2.4.2
Description
Fixes #1421
enhancements
Replaced
raster_calculatorwithraster_mapin places where it simplified things.There are still many places where
raster_calculatoris needed, mostly related to nodata masking. I didn't write full docstrings for the simple one-line functions that are passed toraster_map, hoping that they can be replaced with lambdas in the near future.Replaced kernel creation functions with
pygeoprocessing.kernelswhere possible.Replaced
utils.array_equals_nodatawithpygeoprocessing.array_equals_nodata.changes to tests
A few expected values in the UCM tests had to be updated because of changes to
zonal_statistics. Out of almost 1500 building polygons in the test data, zonal stats results were different (and more correct) for 7 of them. I am guessing because we used to reproject the raster to match the vector; now we reproject the vector to match the raster, which should be more accurate. This probably only showed up in UCM because it uses so many polygons and they are small relative to the pixel size.In one habitat quality test, we were incorrectly creating a raster from an int8 array and then inserting the value 255 into it. Previously this worked because pygeoprocessing incorrectly created a uint8 raster from the int8 array. Now pygeoprocessing copies over the type correctly, and so the 255 became a -1. Fixed by updating the original array dtype.
Checklist