opera-adt / COMPASS

COregistered Multi-temPorAl Sar Slc
Apache License 2.0
39 stars 18 forks source link

WIP Solid Earth tide correction #68

Closed vbrancat closed 1 year ago

vbrancat commented 1 year ago

This PR introduces the Solid Earth tide (SET) correction in the series of LUTs to give as an input to the geocode SLC core module. SET are computed using pySolid which has been introduced as a new dependency in COMPASS

TO DO:

  1. Validate that computed SET are correct. From preliminary testing, I am able to correctly get SET in ENU. However, when converting ENU SET to the radar grid of rdr2geo, the result of scipy.interpolate.RegularGridInterpolator is zero
  2. Double-check that the correct convention and conversion between azimuth angle and orbit azimuth angle has been applied
  3. SET in LOS and azimuth need to be interpolated to the same grid of the other corrections
  4. Convert SET along azimuth from meters to seconds
  5. SET contributions need to be added to the total LUT in range and azimuth direction
  6. Write the SET contribution (and the other correction layers) inside the output product
vbrancat commented 1 year ago

Posting Yunjun's preliminary PR review shared on Slack:

Hi Virginia, I am testing your solid Earth tides branch on COMPASS. After configuring the yaml file, s1_cslc.py went through without error, which is very nice. What’s the output of this step? It seems that it’s computed but not written into files. I would suggest the following to help your own development/validation and communication with others: a. embed a debug_mode code inside compass, to help testing and future diagnose. I have a lot of this in mintpy (https://github.com/insarlab/MintPy/blob/0b0a8eb6c6e4ac1945398e378bc881bbfd099b0e/src/mintpy/plate_motion.py#L113-L130). We could discuss this in the CSLC tag-up, or you may bring this up in the ADT meeting (I won’t attend ADT meetings until after AGU due to a time conflict with a class). OR b. have a jupyter notebook uploaded to our notebook repo. I actually prefer option a, as it’s easier. Attached screenshot is the output range and azimuth lut value. Both are not reasonable yet, because:

  1. the output unit of pysolid is meter, which need to be converted into our lut unit, which are pixel or second?
  2. theen2az() is not right yet, check here (https://github.com/insarlab/MintPy/blob/main/src/mintpy/utils/utils0.py#L621) please. The LOS azimuth angle and orbit azimuth angle are different, see the definitions here (https://github.com/insarlab/MintPy/blob/main/src/mintpy/utils/utils0.py#L531). Because of this easy confusion (to us and to the future users for sure), I suggest we start to document all the geometry and sign conventions in the github wiki (https://github.com/opera-adt/COMPASS/wiki), starting from this one. After fixing the two, I think we may be good to go for the PR.

Below are two more minor suggestions:

  1. In the scratch dir for the solid_earth_tides directory, the heading.rdr file is actually the LOS azimuth angle, right? If so, I would suggest to use azimuth_angle.rdr instead, as HEADING angle is technically different from LOS azimuth angle.
    1. I would suggest to use incidence_angle.rdr for the filename as well, to be sure of clarity.
vbrancat commented 1 year ago

Answering Yunjun's comments:

  1. Debugging statements are actually a good idea but since we are running a bit out of time with the Beta delivery, I would prefer focusing on the theoretical part of the SET computation and whether this is done right. We can always submit an issue and get to it when we have time
  2. You are correct, we need to convert the SET azimuth contribution from meters to seconds before summing it up to the overall azimuth LUT which is the input to the geocode SLC core module.
  3. You are right, I think in the comments I explicitly indicated the convention used for the various declination of azimuth angles. I introduced this function https://github.com/vbrancat/COMPASS/blob/312fd507a5c4627e0c20e1bb33deca0c88ff90d0/src/compass/utils/lut.py#L253 to do the conversion from what we call "heading angle" and to the definition of azimuth angle needed to project the SET contribution from enu to LOS and azimuth directions. I would appreciate if you can double-check
  4. It would be not correct to change the name of heading.rdr into azimuth_angle.rdr as its definition differs from that of the community. I reject this comment
  5. Changed the name of incidence.rdr in incidence_angle.rdr
vbrancat commented 1 year ago

Closing because branch is stale and takes too much time to resolve conflicts. Will open a new PR.