ACCESS-Cloud-Based-InSAR / dem-stitcher

Download and merge DEM tiles
Apache License 2.0
39 stars 15 forks source link

add parameter `merge_respect_extent` #82

Closed kidanger closed 2 months ago

kidanger commented 2 months ago

Hello,

This PR adds a parameter to change the behavior of the merge step when tiles are missing. This is related to https://github.com/ACCESS-Cloud-Based-InSAR/dem-stitcher/issues/62.

What do you think?

cmarshak commented 2 months ago

Thank you for your interest in improving this library.

So, unfortunately, there is a problem with this approach that I try to describe in the readme that we saw in the early development of this library. Because we use the AREA_OR_POINT tag for georeferencing, which in itself is a controversial topic (see [1, 2, 3]), we do translations of the rasters. However, if the merge function allows for resampling, which by passing extents we are changing the origin, then we will get unexpected results particularly if we request translations downstream by asking a DEM with tag POINT to be converted to AREA or vice versa.

For the time being, I would just handle this using rasterio directly defining a suitable profile for application. Know this might be slightly more work, but I rather be as consistent as possible in this library at the expense of some ease of use.

If we really wanted to implement this for the library the way it is written, then one would want to manually define a profile that included the input extents such that the new output extents were smallest extents that were an integer resolution distance from the input extents of the merged DEM so that no resampling is performed (just would write the merged DEM pixels into their respective positions). However, the accounting and testing this is a huge lift.

Hope this all makes sense.

References: [1] https://trac.osgeo.org/gdal/ticket/3837 [2] https://github.com/opengeospatial/ogcapi-coverages/issues/92 [3] https://gdal.org/user/raster_data_model.html#metadata