opera-adt / COMPASS

COregistered Multi-temPorAl Sar Slc
Apache License 2.0
38 stars 17 forks source link

add a flag to indicate if elevation antenna pattern (EAP) correction #175

Closed seongsujeong closed 12 months ago

seongsujeong commented 1 year ago

This PR is the same one as #174, but after cleaning the upstream commit history in the source branch.

Below is the description that I've put in the PR #174:

This PR adds a flag correction_applied_by_sas into /metadata/processing_information/parameters/elevation_antenna_pattern_correction to indicate if the elevation antenna pattern (EAP) correction was applied by SAS (i.e. COMPASS). Also the EAP information is moved from /metadata/processing_information/timing_corrections to /metadata/processing_information/parameter

seongsujeong commented 1 year ago

@seongsujeong thank you for this PR. I am not sure I fully understand the implementation. To my understanding:

  1. If we apply antenna pattern correction, then we allocate a coarse 2D LUT inside /metadata/processing_information/timing_corrections, correct? I am assuming this was not done before

EAP correction is not a timing correction. It is a correction for phase and/or amplitude ( very old data). The LUT to apply the EAP correction is named vec_eap_line in here.It is a 1-D LUT with complex numbers, whose length is the same as the number of samples in radargrid. So it's very small. Currently we do not save it into CSLC product, but I think it shouldn't be tricky. Do you have any suggestion about where the EAP LUT be located?

  1. I am not sure what correction_applied_by_sas is and what it should represent. Can you clarify?

Also, I think we should add a metadata field in metadata/processing_information/parameters being is_antenna_pattern_correction_applied (or whichever is the convention we selected to name metadata)

correction_applied_by_sas is to clarify that the correction was applied by this SAS (i.e. s1-reader and COMPASS). ESA has started to apply EAP correction from IPF > 2.43 by default. Therefore it would be ESA who do the correction in that case. So I think it would be more informative and useful if we clarify who applied the correction (OPERA SAS or ESA) in case we find issue from the correction. Suggestion for the field name is welcomed. Maybe we can populate the field with the string COMPASS or ESA rather than storing the flag.

vbrancat commented 1 year ago

@seongsujeong thank you for this PR. I am not sure I fully understand the implementation. To my understanding:

  1. If we apply antenna pattern correction, then we allocate a coarse 2D LUT inside /metadata/processing_information/timing_corrections, correct? I am assuming this was not done before

EAP correction is not a timing correction. It is a correction for phase and/or amplitude ( very old data). The LUT to apply the EAP correction is named vec_eap_line in here.It is a 1-D LUT with complex numbers, whose length is the same as the number of samples in radargrid. So it's very small. Currently we do not save it into CSLC product, but I think it shouldn't be tricky. Do you have any suggestion about where the EAP LUT be located?

  1. I am not sure what correction_applied_by_sas is and what it should represent. Can you clarify?

Also, I think we should add a metadata field in metadata/processing_information/parameters being is_antenna_pattern_correction_applied (or whichever is the convention we selected to name metadata)

correction_applied_by_sas is to clarify that the correction was applied by this SAS (i.e. s1-reader and COMPASS). ESA has started to apply EAP correction from IPF > 2.43 by default. Therefore it would be ESA who do the correction in that case. So I think it would be more informative and useful if we clarify who applied the correction (OPERA SAS or ESA) in case we find issue from the correction. Suggestion for the field name is welcomed. Maybe we can populate the field with the string COMPASS or ESA rather than storing the flag.

@seongsujeong, sorry, my misunderstanding. No need to save the EAP correction in the product but double-check if @hfattahi has a preference on this matter.

I doubt that people know what SAS means XD. Do we need to differentiate if it us or ESA that performs the antenna pattern correction? It might be transparent to mention that we do it but this is unnecessary nit-picking IMO.

I would just have a flag is_antenna_pattern_correction_applied in metadata/processing_information/parameters. Will add this as a topic for our CSLC-S1 meeting

seongsujeong commented 1 year ago

@vbrancat @hfattahi COMPASS will always use EAP-corrected SLC. In case ESA does not correct EAP, we correct the data using our implementation of their algorithm. That being said, the field is_antenna_pattern_correction_applied will always be True, so I am not sure how meaningful this would be. Instead, what I think it would be useful is to keep track of who did the correction. In case we find some issues in EAP correction (e.g. incorrect correction), we will be able to trace back if the problem is coming from our EAP correction implementation, or somewhere else. I'm happy to discuss this in CSLC meeting.

I doubt that people know what SAS means XD.

Totally agreed. I have a horrible sense of metadata field naming LOL Let's discuss about it in the meeting as well.

seongsujeong commented 12 months ago

Looks like the failure in circleCI comes from changes in the location of types.py which will be take care of when we have release cut on ISCE3 side.

ImportError while importing test module '/home/compass_user/OPERA/COMPASS/tests/test_s1_geocode_slc.py'. Hint: make sure your test modules/packages have valid Python names. Traceback: ../../miniconda3/envs/COMPASS/lib/python3.11/importlib/init.py:126: in import_module return _bootstrap._gcd_import(name[level:], package, level) tests/test_s1_geocode_slc.py:8: in from compass import s1_geocode_slc ../../miniconda3/envs/COMPASS/lib/python3.11/site-packages/compass/s1_geocode_slc.py:34: in from isce3.core.types import (truncate_mantissa, to_complex32) E ModuleNotFoundError: No module named 'isce3.core.types