ngageoint / sarpy

A basic Python library to demonstrate reading, writing, display, and simple processing of complex SAR data using the NGA SICD standard.
MIT License
262 stars 87 forks source link

AMP_SF defined for float32 cphd data type causes error in cphd reader; should be ignored for non integer cphd. #552

Closed jdgorman8r closed 1 month ago

jdgorman8r commented 1 month ago

AMP_SF is used to allow pulse-to-pulse scaling for pulse data that is stored as int8 or int16. It is irrelevant for float32 format data. Nevertheless, some CPHDs define an AMP_SF factor even though the pulse raw_data type is float32. This results in an error in cphd.py and the cphd reader. If raw_data is float32, an alternative would be to set the amplitude scaling factor to None. See the example below (this was suggested by Jeff Harrington at SRI). This was our fix:

cphd.py line 126-130:

NB: more validation as part of validate_shapes

      if self._raw_dtype.name not in ['int8', 'int16']:             raise ValueError(                   'A scaling multiplier has been supplied,\n\t'                   'but the raw datatype is not int8 or int16.')       self._amplitude_scaling = array       self._validate_amplitude_scaling()

becomes:

NB: more validation as part of validate_shapes

      if self._raw_dtype.name not in ['int8', 'int16']:       #        raise ValueError(       #            'A scaling multiplier has been supplied,\n\t'       #            'but the raw datatype is not int8 or int16.')       print('cphd.AmplitudeScalingFunction: '             'A scaling multiplier has been supplied,\n\t'             'but the raw datatype is not 'int8' or 'int16'.\n'             'Setting amplitude_scaling to None.')       self._amplitude_scaling = None       self._validate_amplitude_scaling()

pressler-vsc commented 1 month ago

Thanks @jdgorman8r. Similar to this comment from #551, I believe this may have been addressed in integration/1.3.59-rc via #480. If you are able, can you see if you encounter an issue in integration/1.3.59-rc?

jdgorman8r commented 1 month ago

Daniel, excellent. Thanks! Looks like 1.3.59-rc fixes the issue. Thanks for the info. We'll work with that version