Skip to content

Fix complex int16 dn dtype slc #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

agrouaze
Copy link
Member

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: pydata/xarray#5491

Copy link
Collaborator

@oarcher oarcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice !
Can you use a dict mapping instead of an 'if' ?

@@ -433,7 +432,11 @@ def _load_digital_number(self, resolution=None, chunks=None, resampling=rasterio
# sort chunks keys like map_dims
chunks = dict(sorted(chunks.items(), key=lambda pair: list(map_dims.keys()).index(pair[0])))
chunks_rio = {map_dims[d]: chunks[d] for d in map_dims.keys()}

riot_dtyp = rio.dtypes[0]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Grouny , can you replace with:

try:
  rio_dtyp = self._rio_dtypes[rio.dtypes[0]]
except KeyError:
  rio_dtyp  = rio.dtypes[0]

and define:

self._rio_dtypes = {'complex_int16' : 'complex'}

somewhere in init ?

@@ -479,11 +482,13 @@ def _load_digital_number(self, resolution=None, chunks=None, resampling=rasterio
rio.height // resolution['atrack'] * resolution['atrack'])

if self.s1meta.driver == 'GTiff':
logging.info('rio dtypes zero: %s',rio.dtypes[0])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be removed if it's works as expected.

@agrouaze
Copy link
Member Author

agrouaze commented Sep 20, 2021

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 (corteva/rioxarray#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
Copy link
Collaborator

oarcher commented Sep 20, 2021

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 (corteva/rioxarray#376)

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

@agrouaze
Copy link
Member Author

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
Copy link
Collaborator

oarcher commented Apr 9, 2022

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

@agrouaze
Copy link
Member Author

Ok to close this PR without merge.

@agrouaze agrouaze closed this Apr 11, 2022
@agrouaze agrouaze deleted the fix_complex_int16_DN_dtype_slc branch May 27, 2024 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants