search

umr-lops / xsar

Synthetic Aperture Radar (SAR) Level-1 GRD python mapper for efficient xarray/dask based processing
https://cyclobs.ifremer.fr/static/sarwing_datarmor/xsar/
MIT License
26 stars 8 forks source link

Fix complex int16 dn dtype slc #5

Closed agrouaze closed 2 years ago

agrouaze commented 3 years ago

small fix to allow rasterio complex_int16 to be interpreted as np.complex_ in order to fix the dtype casting error in case of coarse resolution options on SLC (complex Digital Number). For instance xsar.open_dataset(wv_slc_meta.subdatasets[good_indice],resolution=resolutionZ,resampling=rasterio.enums.Resampling.rms) is now possible. Note that rasterio will be probably replaced by rioxarray in xarray backends: https://github.com/pydata/xarray/issues/5491

agrouaze commented 2 years ago

Hi, With Thomas, we came back on this bug for complex values in xsar/xarray/rasterio. Thanks to https://github.com/keewis , we tested a new syntax 

xr.open_dataset(f,chunks=chunks_rio, parse_coordinates=False,engine='rasterio')

this syntax benefits from rioarray fixes (https://github.com/corteva/rioxarray/pull/359 ) but current syntax in xsar:

xr.open_rasterio(f, chunks=chunks_rio, parse_coordinates=False)

is not working for SLC data. So I think the current PR is useless and that we have to opt for this new way to open datasets. What do you think @oarcher ?

oarcher commented 2 years ago

Yes, it would be nice to drop xr.open_rasterio in favor of rioxarray in xsar. But GCPs are not well handled by rioxarray for the moment, so we have to wait for some improvements (https://github.com/corteva/rioxarray/issues/376)

So for now, I think that this PR is the best solution.

agrouaze commented 2 years ago

Going back on this PR, I cannot find the internal types to decode the .tiff files for SLC products since it is now integrated in the rioxarray library. The problem is that we don't get the same numbers than cerbere for real nor imaginary part of the digital_number. It needs a discussion to understand where the changes have to be done.

oarcher commented 2 years ago

It's seems that this problem has been fixed upstream in rasterio. @agrouaze , are you ok to close this PR without merge ?

agrouaze commented 2 years ago

Ok to close this PR without merge.